| Message ID | 20180925144127.31965-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virt: Suppress external aborts on virt-2.10 and earlier | expand |
Hi Peter, On 9/25/18 4:41 PM, Peter Maydell wrote: > In commit c79c0a314c43b78 we enabled emulation of external aborts > when the guest attempts to access a physical address with no > mapped device. In commit 4672cbd7bed88dc6 we suppress this for > most legacy boards to prevent breakage of previously working > guests, but we didn't suppress it in the 'virt' board, with > the rationale "we know that guests won't try to prod devices > that we don't describe in the device tree or ACPI tables". This > is mostly true, but we've had a report of a Linux guest image > that this did break. The problem seems to be that the guest > is (incorrectly) configured with a DEBUG_UART_PHYS value that > tells it there is a uart at 0x10009000 (which is true for > vexpress but not for virt), so in early bootup the kernel > probes this bogus address. > > This is a misconfigured guest, so we don't need to worry > about it too much, but we can arrange that guests that ran > on QEMU v2.10 (before c79c0a314c43b78) will still run on > the "virt-2.10" board model, by suppressing external aborts > only for that version and earlier. This seems a reasonable > compromise. I tried another approach to keep MEMTX_DECODE_ERROR delivered (out of 0x10009000) for 2.10, using the UNIMP device, but the code looks uglier (create too much generic code for a single issue): -- >8 -- diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 4cc57a7ef6..217c9de6f6 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -101,6 +101,7 @@ typedef struct { bool claim_edge_triggered_timers; bool smbios_old_sys_ver; bool no_highmem_ecam; + bool misconfigured_guest_debug_uart; } VirtMachineClass; typedef struct { diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 0b57f87abc..29bc9b9feb 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -59,6 +59,7 @@ #include "qapi/visitor.h" #include "standard-headers/linux/input.h" #include "hw/arm/smmuv3.h" +#include "hw/misc/unimp.h" #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \ static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ @@ -1532,6 +1533,10 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(0)); + if (vmc->misconfigured_guest_debug_uart) { + /* kludge ... */ + create_unimplemented_device("phantom-pl011", 0x10009000, 0x1000); + } if (vms->secure) { create_secure_ram(vms, secure_sysmem); @@ -1924,8 +1929,11 @@ static void virt_2_10_instance_init(Object *obj) static void virt_machine_2_10_options(MachineClass *mc) { + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc)); + virt_machine_2_11_options(mc); SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_10); + vmc->misconfigured_guest_debug_uart = true; } DEFINE_VIRT_MACHINE(2, 10) -- I prefer your compromise. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm/virt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 0b57f87abcb..3ba310a37b6 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1926,6 +1926,7 @@ static void virt_machine_2_10_options(MachineClass *mc) > { > virt_machine_2_11_options(mc); > SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_10); Can you add a short comment here? /* See https://bugs.launchpad.net/qemu/+bug/???: * Some misconfigured Linux guest poke for vexpress uart * at 0x10009000 */ Regardless: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + mc->ignore_memory_transaction_failures = true; > } > DEFINE_VIRT_MACHINE(2, 10) >
On 26 September 2018 at 11:35, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Peter, > > On 9/25/18 4:41 PM, Peter Maydell wrote: >> In commit c79c0a314c43b78 we enabled emulation of external aborts >> when the guest attempts to access a physical address with no >> mapped device. In commit 4672cbd7bed88dc6 we suppress this for >> most legacy boards to prevent breakage of previously working >> guests, but we didn't suppress it in the 'virt' board, with >> the rationale "we know that guests won't try to prod devices >> that we don't describe in the device tree or ACPI tables". This >> is mostly true, but we've had a report of a Linux guest image >> that this did break. The problem seems to be that the guest >> is (incorrectly) configured with a DEBUG_UART_PHYS value that >> tells it there is a uart at 0x10009000 (which is true for >> vexpress but not for virt), so in early bootup the kernel >> probes this bogus address. >> >> This is a misconfigured guest, so we don't need to worry >> about it too much, but we can arrange that guests that ran >> on QEMU v2.10 (before c79c0a314c43b78) will still run on >> the "virt-2.10" board model, by suppressing external aborts >> only for that version and earlier. This seems a reasonable >> compromise. > > I tried another approach to keep MEMTX_DECODE_ERROR delivered (out of > 0x10009000) for 2.10, using the UNIMP device, but the code looks uglier > (create too much generic code for a single issue): I definitely don't want to do that... > I prefer your compromise. > >> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> hw/arm/virt.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 0b57f87abcb..3ba310a37b6 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -1926,6 +1926,7 @@ static void virt_machine_2_10_options(MachineClass *mc) >> { >> virt_machine_2_11_options(mc); >> SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_10); > > Can you add a short comment here? > > /* See https://bugs.launchpad.net/qemu/+bug/???: > * Some misconfigured Linux guest poke for vexpress uart > * at 0x10009000 */ I view it as more "virt-2.10 is supposed to behave like the 2.10 release virt" rather than a specific fix for this guest, I think. thanks -- PMM
On 9/26/18 2:29 PM, Peter Maydell wrote: > On 26 September 2018 at 11:35, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Hi Peter, >> >> On 9/25/18 4:41 PM, Peter Maydell wrote: >>> In commit c79c0a314c43b78 we enabled emulation of external aborts >>> when the guest attempts to access a physical address with no >>> mapped device. In commit 4672cbd7bed88dc6 we suppress this for >>> most legacy boards to prevent breakage of previously working >>> guests, but we didn't suppress it in the 'virt' board, with >>> the rationale "we know that guests won't try to prod devices >>> that we don't describe in the device tree or ACPI tables". This >>> is mostly true, but we've had a report of a Linux guest image >>> that this did break. The problem seems to be that the guest >>> is (incorrectly) configured with a DEBUG_UART_PHYS value that >>> tells it there is a uart at 0x10009000 (which is true for >>> vexpress but not for virt), so in early bootup the kernel >>> probes this bogus address. >>> >>> This is a misconfigured guest, so we don't need to worry >>> about it too much, but we can arrange that guests that ran >>> on QEMU v2.10 (before c79c0a314c43b78) will still run on >>> the "virt-2.10" board model, by suppressing external aborts >>> only for that version and earlier. This seems a reasonable >>> compromise. >> >> I tried another approach to keep MEMTX_DECODE_ERROR delivered (out of >> 0x10009000) for 2.10, using the UNIMP device, but the code looks uglier >> (create too much generic code for a single issue): > > I definitely don't want to do that... > >> I prefer your compromise. >> >>> >>> Cc: qemu-stable@nongnu.org >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> --- >>> hw/arm/virt.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 0b57f87abcb..3ba310a37b6 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -1926,6 +1926,7 @@ static void virt_machine_2_10_options(MachineClass *mc) >>> { >>> virt_machine_2_11_options(mc); >>> SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_10); >> >> Can you add a short comment here? >> >> /* See https://bugs.launchpad.net/qemu/+bug/???: >> * Some misconfigured Linux guest poke for vexpress uart >> * at 0x10009000 */ > > I view it as more "virt-2.10 is supposed to behave like > the 2.10 release virt" rather than a specific fix for > this guest, I think. You right, fine. Very useful short explanation btw.
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 0b57f87abcb..3ba310a37b6 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1926,6 +1926,7 @@ static void virt_machine_2_10_options(MachineClass *mc) { virt_machine_2_11_options(mc); SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_10); + mc->ignore_memory_transaction_failures = true; } DEFINE_VIRT_MACHINE(2, 10)
In commit c79c0a314c43b78 we enabled emulation of external aborts when the guest attempts to access a physical address with no mapped device. In commit 4672cbd7bed88dc6 we suppress this for most legacy boards to prevent breakage of previously working guests, but we didn't suppress it in the 'virt' board, with the rationale "we know that guests won't try to prod devices that we don't describe in the device tree or ACPI tables". This is mostly true, but we've had a report of a Linux guest image that this did break. The problem seems to be that the guest is (incorrectly) configured with a DEBUG_UART_PHYS value that tells it there is a uart at 0x10009000 (which is true for vexpress but not for virt), so in early bootup the kernel probes this bogus address. This is a misconfigured guest, so we don't need to worry about it too much, but we can arrange that guests that ran on QEMU v2.10 (before c79c0a314c43b78) will still run on the "virt-2.10" board model, by suppressing external aborts only for that version and earlier. This seems a reasonable compromise. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 1 + 1 file changed, 1 insertion(+) -- 2.19.0