[Qemu-devel] [PATCH] hw/arm/boot: Increase compliance with kernel arm64 boot protocol.
"The Image must be placed text_offset bytes from a 2MB aligned base
address anywhere in usable system RAM and called there."
For the virt board, we write our startup bootloader at the very
bottom of RAM, so that bit can't be used for the image. To avoid
overlap in case the image requests to be loaded at an offset
smaller than our bootloader, we increment the load offset to the
next 2MB.
This fixes a boot failure for Xen AArch64.
Signed-off-by: Stewart Hildebrand
---
hw/arm/boot.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 20c71d7d96..559ddbcd53 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -919,6 +919,16 @@ static uint64_t load_aarch64_image(const char *filename,
hwaddr mem_base,
memcpy(&hdrvals, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(hdrvals));
if (hdrvals[1] != 0) {
kernel_load_offset = le64_to_cpu(hdrvals[0]);
+
+/* For the virt board, we write our startup "bootloader" at the
very
+ * bottom of RAM, so that bit can't be used for the image. To avoid
+ * overlap in case the image requests to be loaded at an offset
+ * smaller than our bootloader, we increment the load offset to the
+ * next 2MB.
+ */
+if (kernel_load_offset < FIXUP_MAX) {
+kernel_load_offset += 2 << 20;
+}
}
}
--
2.17.1
Re: [Qemu-devel] [PATCH] hw/arm/boot: Increase compliance with kernel arm64 boot protocol.
Hi Philippe,
On Monday, October 15, 2018 6:05 PM, Philippe Mathieu-Daudé wrote:
> Hi Stewart,
>
> On 15/10/2018 23:26, Stewart Hildebrand wrote:
> > +/* For the virt board, we write our startup "bootloader" at
> > the very
> > + * bottom of RAM, so that bit can't be used for the image. To
> > avoid
> > + * overlap in case the image requests to be loaded at an offset
> > + * smaller than our bootloader, we increment the load offset
> > to the
> > + * next 2MB.
> > + */
> > +if (kernel_load_offset < FIXUP_MAX) {
>
> I don't understand how this is related to FIXUP_MAX...
You're right, my apologies, it's not directly related.
write_bootloader() calculates the size of the bootloader like so:
len = 0;
while (insns[len].fixup != FIXUP_TERMINATOR) {
len++;
}
The size of the bootloader then would be len * sizeof(uint32_t)
It would be nice not to have to repeat that logic in load_aarch64_image(). I'll
send out a v2 after I take some time to wrap my head around it...
>
> > +kernel_load_offset += 2 << 20;
>
> You can use += 2 * MiB; which is easier to review.
OK, I will include this in v2.
[Qemu-devel] [PATCH v2] hw/arm/boot: Increase compliance with kernel arm64 boot protocol
"The Image must be placed text_offset bytes from a 2MB aligned base
address anywhere in usable system RAM and called there."
For the virt board, we write our startup bootloader at the very
bottom of RAM, so that bit can't be used for the image. To avoid
overlap in case the image requests to be loaded at an offset
smaller than our bootloader, we increment the load offset to the
next 2MB.
This fixes a boot failure for Xen AArch64.
Signed-off-by: Stewart Hildebrand
---
Changes v1 -> v2:
- use KiB/MiB macros for readability (suggested by Philippe Mathieu-Daudé),
hence the additional #include
- define an upper bound for the bootloader size since TEXT_OFFSET has to be
page aligned anyway (suggested by Andre Przywara)
- add assert() in write_bootloader() to make sure we stay below the 4K max
(suggested by Peter Maydell)
---
hw/arm/boot.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 20c71d7d96..a675a602bc 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -24,6 +24,7 @@
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "exec/address-spaces.h"
+#include "qemu/units.h"
/* Kernel boot protocol is specified in the kernel docs
* Documentation/arm/Booting and Documentation/arm64/booting.txt
@@ -36,6 +37,8 @@
#define ARM64_TEXT_OFFSET_OFFSET8
#define ARM64_MAGIC_OFFSET 56
+#define BOOTLOADER_MAX_SIZE (4 * KiB)
+
AddressSpace *arm_boot_address_space(ARMCPU *cpu,
const struct arm_boot_info *info)
{
@@ -184,6 +187,8 @@ static void write_bootloader(const char *name, hwaddr addr,
code[i] = tswap32(insn);
}
+assert((len * sizeof(uint32_t)) < BOOTLOADER_MAX_SIZE);
+
rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
g_free(code);
@@ -919,6 +924,16 @@ static uint64_t load_aarch64_image(const char *filename,
hwaddr mem_base,
memcpy(&hdrvals, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(hdrvals));
if (hdrvals[1] != 0) {
kernel_load_offset = le64_to_cpu(hdrvals[0]);
+
+/* For the virt board, we write our startup "bootloader" at the
very
+ * bottom of RAM, so that bit can't be used for the image. To avoid
+ * overlap in case the image requests to be loaded at an offset
+ * smaller than our bootloader, we increment the load offset to the
+ * next 2MB.
+ */
+if (kernel_load_offset < BOOTLOADER_MAX_SIZE) {
+kernel_load_offset += 2 * MiB;
+}
}
}
--
2.17.1
RE: [Qemu-arm] [RFC PATCH 00/14] hw/arm: Add the Raspberry Pi 4B
On Wednesday, September 4, 2019 1:13 PM, Philippe Mathieu-Daudé wrote: >Esteban wrote me over the weekend asking about raspi4 progress. >I cleaned up my patches/notes to pass him. Other help is also welcomed :) >I got scared trying to understand how to use the GIC, and wire the various >IRQs. > >Most important notes about testing are in patch #12: >"Add the BCM2838 which uses a GICv2" > >Not much works yet, it only runs a bit until configuring the GIC. > >branch pushed at https://gitlab.com/philmd/qemu/commits/raspi4_wip > >Regards, > >Phil. It seems upstream linux may be adopting the BCM2711 naming convention [1], though it doesn't look like the series [1] has been committed yet. The SoC name in documentation is BCM2711 [2]. I have no opinion on which naming convention the QEMU community adopts, I simply wanted to pass along this observation. -Stew [1] https://patchwork.kernel.org/cover/11092613/ [2] https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/README.md
Re: [RFC QEMU PATCH v7 1/1] xen/pci: get gsi for passthrough devices
+Edgar
On 5/16/24 06:13, Jiqian Chen wrote:
> In PVH dom0, it uses the linux local interrupt mechanism,
> when it allocs irq for a gsi, it is dynamic, and follow
> the principle of applying first, distributing first. And
> the irq number is alloced from small to large, but the
> applying gsi number is not, may gsi 38 comes before gsi
> 28, that causes the irq number is not equal with the gsi
> number. And when passthrough a device, qemu wants to use
> gsi to map pirq, xen_pt_realize->xc_physdev_map_pirq, but
> the gsi number is got from file
> /sys/bus/pci/devices//irq in current code, so it
> will fail when mapping.
>
> Get gsi by using new function supported by Xen tools.
>
> Signed-off-by: Huang Rui
> Signed-off-by: Jiqian Chen
I think you can safely remove the RFC tag since the Xen bits have been
upstreamed.
> ---
> hw/xen/xen-host-pci-device.c | 19 +++
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 8c6e9a1716a2..2fe6a60434ba 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -10,6 +10,7 @@
> #include "qapi/error.h"
> #include "qemu/cutils.h"
> #include "xen-host-pci-device.h"
> +#include "hw/xen/xen_native.h"
The inclusion order unfortunately seems to be delicate.
"hw/xen/xen_native.h" should be before all the other xen
includes, but after "qemu/osdep.h".
>
> #define XEN_HOST_PCI_MAX_EXT_CAP \
> ((PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) / (PCI_CAP_SIZEOF + 4))
> @@ -329,12 +330,17 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice
> *d, uint32_t cap)
> return -1;
> }
>
> +#define PCI_SBDF(seg, bus, dev, func) \
> +uint32_t)(seg)) << 16) | \
> +(PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func
> +
> void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
> uint8_t bus, uint8_t dev, uint8_t func,
> Error **errp)
> {
> ERRP_GUARD();
> unsigned int v;
> +uint32_t sdbf;
Typo: s/sdbf/sbdf/
>
> d->config_fd = -1;
> d->domain = domain;
> @@ -364,11 +370,16 @@ void xen_host_pci_device_get(XenHostPCIDevice *d,
> uint16_t domain,
> }
> d->device_id = v;
>
> -xen_host_pci_get_dec_value(d, "irq", &v, errp);
> -if (*errp) {
> -goto error;
> +sdbf = PCI_SBDF(domain, bus, dev, func);
> +d->irq = xc_physdev_gsi_from_dev(xen_xc, sdbf);
This was renamed to xc_pcidev_get_gsi.
This also needs some sort of Xen interface version guard for backward
compatibility since it's a new call introduced in Xen 4.20.
> +/* fail to get gsi, fallback to irq */
> +if (d->irq == -1) {
> +xen_host_pci_get_dec_value(d, "irq", &v, errp);
> +if (*errp) {
> +goto error;
> +}
> +d->irq = v;
> }
> -d->irq = v;
>
> xen_host_pci_get_hex_value(d, "class", &v, errp);
> if (*errp) {
Re: [QEMU PATCH v8] xen/passthrough: use gsi to map pirq when dom0 is PVH
On 10/16/24 02:28, Jiqian Chen wrote:
> In PVH dom0, when passthrough a device to domU, QEMU code
> xen_pt_realize->xc_physdev_map_pirq wants to use gsi, but in current codes
> the gsi number is got from file /sys/bus/pci/devices//irq, that is
> wrong, because irq is not equal with gsi, they are in different spaces, so
> pirq mapping fails.
>
> To solve above problem, use new interface of Xen, xc_pcidev_get_gsi to get
> gsi and use xc_physdev_map_pirq_gsi to map pirq when dom0 is PVH.
>
> Signed-off-by: Jiqian Chen
> Signed-off-by: Huang Rui
> Signed-off-by: Jiqian Chen
> ---
> Hi All,
> This is v8 to support passthrough on Xen when dom0 is PVH.
> v7->v8 change:
> * Since xc_physdev_gsi_from_dev was renamed to xc_pcidev_get_gsi, changed it.
> * Added xen_run_qemu_on_hvm to check if Qemu run on PV dom0, if not use
> xc_physdev_map_pirq_gsi to map pirq.
> * Used CONFIG_XEN_CTRL_INTERFACE_VERSION to wrap the new part for
> compatibility.
> * Added "#define DOMID_RUN_QEMU 0" to represent the id of domain that Qemu
> run on.
>
>
> Best regards,
> Jiqian Chen
>
>
>
> v6->v7 changes:
> * Because the function of obtaining gsi was changed on the kernel and Xen
> side. Changed to use
> xc_physdev_gsi_from_dev, that requires passing in sbdf instead of irq.
>
> v5->v6 changes:
> * Because the function of obtaining gsi was changed on the kernel and Xen
> side. Changed to use
> xc_physdev_gsi_from_irq, instead of gsi sysfs.
> * Since function changed, removed the Review-by of Stefano.
>
> v4->v5 changes:
> * Added Review-by Stefano.
>
> v3->v4 changes:
> * Added gsi into struct XenHostPCIDevice and used gsi number that read from
> gsi sysfs
> if it exists, if there is no gsi sysfs, still use irq.
>
> v2->v3 changes:
> * Due to changes in the implementation of the second patch on kernel
> side(that adds
> a new sysfs for gsi instead of a new syscall), so read gsi number from the
> sysfs of gsi.
>
> v1 and v2:
> We can record the relation between gsi and irq, then when userspace(qemu) want
> to use gsi, we can do a translation. The third patch of kernel(xen/privcmd:
> Add new syscall
> to get gsi from irq) records all the relations in acpi_register_gsi_xen_pvh()
> when dom0
> initialize pci devices, and provide a syscall for userspace to get the gsi
> from irq. The
> third patch of xen(tools: Add new function to get gsi from irq) add a new
> function
> xc_physdev_gsi_from_irq() to call the new syscall added on kernel side.
> And then userspace can use that function to get gsi. Then
> xc_physdev_map_pirq() will success.
>
> Issues we encountered:
> 1. failed to map pirq for gsi
> Problem: qemu will call xc_physdev_map_pirq() to map a passthrough device's
> gsi to pirq in
> function xen_pt_realize(). But failed.
>
> Reason: According to the implement of xc_physdev_map_pirq(), it needs gsi
> instead of irq,
> but qemu pass irq to it and treat irq as gsi, it is got from file
> /sys/bus/pci/devices/:xx:xx.x/irq in function xen_host_pci_device_get().
> But actually
> the gsi number is not equal with irq. They are in different space.
> ---
> hw/xen/xen_pt.c | 44
> hw/xen/xen_pt.h | 1 +
> 2 files changed, 45 insertions(+)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 3635d1b39f79..7f8139d20915 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -766,6 +766,41 @@ static void xen_pt_destroy(PCIDevice *d) {
> }
> /* init */
>
> +#define PCI_SBDF(seg, bus, dev, func) \
> +uint32_t)(seg)) << 16) | \
> +(PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func
Nit: This macro looks generic and useful. Would it be better defined in
include/hw/pci/pci.h?
> +
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
> +static bool xen_run_qemu_on_hvm(void)
This function name seems to imply "is qemu running on HVM?", but I think
the question we're really trying to answer is whether the pcidev needs
a GSI mapped. How about calling the function "xen_pt_needs_gsi" or
similar?
> +{
> +xc_domaininfo_t info;
> +
> +if (!xc_domain_getinfo_single(xen_xc, DOMID_RUN_QEMU, &info) &&
> +(info.flags & XEN_DOMINF_hvm_guest)) {
I think reading /sys/hypervisor/guest_type would allow you to get the
same information without another hypercall.
> +return true;
> +}
> +
> +return false;
> +}
> +
> +static int xen_map_pirq_for_gsi(PCIDevice *d, int *pirq)
Nit: s/xen_/xen_pt_/
> +{
> +int gsi;
> +XenPCIPassthroughState *s = XEN_PT_DEVICE(d);
> +
> +gsi = xc_pcidev_get_gsi(xen_xc,
> +PCI_SBDF(s->real_device.domain,
> + s->real_device.bus,
> + s->real_device.dev,
> + s->real_device.func));
> +if (gsi >= 0) {
> +return xc_physdev_map_pirq_gsi(xen_xc, xen_domid, gsi, pirq);
> +}
> +
> +return gsi;
> +}
> +#endif
> +
> static void xen_pt_
Re: [QEMU PATCH v9] xen/passthrough: use gsi to map pirq when dom0 is PVH
On 11/4/24 01:03, Chen, Jiqian wrote:
> On 2024/11/1 21:09, Stewart Hildebrand wrote:
>> On 10/24/24 05:06, Jiqian Chen wrote:
>>> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
>>> index 3635d1b39f79..5b10d501d566 100644
>>> --- a/hw/xen/xen_pt.c
>>> +++ b/hw/xen/xen_pt.c
>>> @@ -766,6 +766,50 @@ static void xen_pt_destroy(PCIDevice *d) {
>>> }
>>> /* init */
>>>
>>> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
>>> +static bool xen_pt_need_gsi(void)
>>> +{
>>> +FILE *fp;
>>> +int len;
>>> +char type[10];
>>
>> A brief in-code comment to explain how you arrived at 10 would be
>> appreciated.
> The max number of characters in the description of the "guest_type" is 4
> ("PVH" plus line break).
> I set it to 10 to prevent longer description types in the future.
> Do you have another suggest number?
No, I think 10 is a good choice. I'm just looking for the rationale to
be documented.
Re: [QEMU PATCH v9] xen/passthrough: use gsi to map pirq when dom0 is PVH
On 10/24/24 05:06, Jiqian Chen wrote:
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 3635d1b39f79..5b10d501d566 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -766,6 +766,50 @@ static void xen_pt_destroy(PCIDevice *d) {
> }
> /* init */
>
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
> +static bool xen_pt_need_gsi(void)
> +{
> +FILE *fp;
> +int len;
> +char type[10];
A brief in-code comment to explain how you arrived at 10 would be
appreciated.
> +const char *guest_type = "/sys/hypervisor/guest_type";
> +
> +fp = fopen(guest_type, "r");
> +if (fp == NULL) {
> +error_report("Cannot open %s: %s", guest_type, strerror(errno));
> +return false;
> +}
> +fgets(type, sizeof(type), fp);
Please check the return value of fgets.
> +fclose(fp);
> +
> +len = strlen(type);
Before passing to strlen, is "type" always guaranteed to have a
terminating '\0' character?
> +if (len) {
> +type[len - 1] = '\0';
> +if (!strcmp(type, "PVH")) {
> +return true;
> +}
> +}
> +return false;
> +}
> +
> +static int xen_pt_map_pirq_for_gsi(PCIDevice *d, int *pirq)
> +{
> +int gsi;
> +XenPCIPassthroughState *s = XEN_PT_DEVICE(d);
> +
> +gsi = xc_pcidev_get_gsi(xen_xc,
> +PCI_SBDF(s->real_device.domain,
> + s->real_device.bus,
> + s->real_device.dev,
> + s->real_device.func));
> +if (gsi >= 0) {
> +return xc_physdev_map_pirq_gsi(xen_xc, xen_domid, gsi, pirq);
> +}
> +
> +return gsi;
> +}
> +#endif
> +
> static void xen_pt_realize(PCIDevice *d, Error **errp)
> {
> ERRP_GUARD();
> @@ -847,7 +891,16 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
> goto out;
> }
>
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 42000
> +if (xen_pt_need_gsi()) {
> +rc = xen_pt_map_pirq_for_gsi(d, &pirq);
> +} else {
> +rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
> +}
> +#else
> rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
> +#endif
> +
> if (rc < 0) {
> XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (err:
> %d)\n",
> machine_irq, pirq, errno);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index eb26cac81098..07805aa8a5f3 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -23,6 +23,10 @@ extern bool pci_available;
> #define PCI_SLOT_MAX32
> #define PCI_FUNC_MAX8
>
> +#define PCI_SBDF(seg, bus, dev, func) \
> +uint32_t)(seg)) << 16) | \
> +(PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func
> +
> /* Class, Vendor and Device IDs from Linux's pci_ids.h */
> #include "hw/pci/pci_ids.h"
>
Re: [QEMU PATCH v10] xen/passthrough: use gsi to map pirq when dom0 is PVH
On 11/6/24 01:14, Jiqian Chen wrote: > In PVH dom0, when passthrough a device to domU, QEMU code > xen_pt_realize->xc_physdev_map_pirq wants to use gsi, but in current codes > the gsi number is got from file /sys/bus/pci/devices//irq, that is > wrong, because irq is not equal with gsi, they are in different spaces, so > pirq mapping fails. > > To solve above problem, use new interface of Xen, xc_pcidev_get_gsi to get > gsi and use xc_physdev_map_pirq_gsi to map pirq when dom0 is PVH. > > Signed-off-by: Jiqian Chen > Signed-off-by: Huang Rui > Signed-off-by: Jiqian Chen Reviewed-by: Stewart Hildebrand
