| Message ID | 20250502132441.64723-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] hw/virtio: Introduce CONFIG_VIRTIO_LEGACY to disable legacy support | expand |
On 5/2/25 6:24 AM, Philippe Mathieu-Daudé wrote: > Legacy VirtIO devices don't have their endianness clearly defined. > QEMU infers it taking the endianness of the (target) binary, or, > when a target support switching endianness at runtime, taking the > endianness of the vCPU accessing the device. > Probably it's what you meant, but it is clearly defined by the standard [1]. It's just not fixed. 2.4.3 Legacy Interfaces: A Note on Virtqueue Endianness Note that when using the legacy interface, transitional devices and drivers MUST use the native endian of the guest as the endian of fields and in the virtqueue. This is opposed to little-endian for non-legacy interface as specified by this standard. It is assumed that the host is already aware of the guest endian. [1] https://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-250003 > Devices modelling shouldn't really change depending on a property > of a CPU accessing it. > Devices concerning various targets are aware of the cpu and its properties. > For heterogeneous systems, it is simpler to break such dev <-> cpu > dependency, only allowing generic device models, with no knowledge > of CPU (or DMA controller) accesses. > At this point, we speculated it could be a problem, without really proving it. In case all accesses are done within a given vcpu thread, there is no ambiguity on the current endianness. How about we wait to expose a crash in an heterogeneous system to take a decision? > Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the > current default (enabled). > New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to > the VirtIO version 1 spec. > I think it's a good change, regarding the legacy support. However, if the only goal is to support a custom configuration for heterogeneous emulation, I think it's the wrong direction. In essence, we are working hard right now to remove compile time configuration for various QEMU targets. So seeing a new compile time configuration to solve something looks like the opposite of what we are trying to achive. A possible alternative would be to enable virtio legacy support at runtime, based on a specific criteria. From what I remember, legacy vs modern is a property set by disable-modern=bool,disable-legacy=bool, and "modern" is the default. Thus, we can simply restrict the disable-legacy=on in case we detect it's an heterogeneous emulation scenario. > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/virtio/virtio-access.h | 5 ++++- > hw/virtio/virtio.c | 8 ++++++++ > target/arm/cpu.c | 5 +++++ > target/ppc/cpu_init.c | 5 +++++ > hw/virtio/Kconfig | 5 +++++ > 5 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h > index 07aae69042a..b5b471711a6 100644 > --- a/include/hw/virtio/virtio-access.h > +++ b/include/hw/virtio/virtio-access.h > @@ -20,7 +20,10 @@ > #include "hw/virtio/virtio.h" > #include "hw/virtio/virtio-bus.h" > > -#if defined(TARGET_PPC64) || defined(TARGET_ARM) > +#include CONFIG_DEVICES > + > +#if defined(CONFIG_VIRTIO_LEGACY) && \ > + (defined(TARGET_PPC64) || defined(TARGET_ARM)) > #define LEGACY_VIRTIO_IS_BIENDIAN 1 > #endif > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 480c2e50365..659ab3cb969 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -47,6 +47,8 @@ > #include "standard-headers/linux/virtio_mem.h" > #include "standard-headers/linux/virtio_vsock.h" > > +#include CONFIG_DEVICES > + > /* > * Maximum size of virtio device config space > */ > @@ -3502,6 +3504,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t device_id, size_t config_size) > bool virtio_legacy_allowed(VirtIODevice *vdev) > { > switch (vdev->device_id) { > +#ifdef CONFIG_VIRTIO_LEGACY > case VIRTIO_ID_NET: > case VIRTIO_ID_BLOCK: > case VIRTIO_ID_CONSOLE: > @@ -3513,6 +3516,7 @@ bool virtio_legacy_allowed(VirtIODevice *vdev) > case VIRTIO_ID_RPROC_SERIAL: > case VIRTIO_ID_CAIF: > return true; > +#endif > default: > return false; > } > @@ -4014,8 +4018,10 @@ static const Property virtio_properties[] = { > DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features), > DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true), > DEFINE_PROP_BOOL("use-disabled-flag", VirtIODevice, use_disabled_flag, true), > +#ifdef CONFIG_VIRTIO_LEGACY > DEFINE_PROP_BOOL("x-disable-legacy-check", VirtIODevice, > disable_legacy_check, false), > +#endif > }; > > static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev) > @@ -4151,7 +4157,9 @@ static void virtio_device_class_init(ObjectClass *klass, const void *data) > vdc->start_ioeventfd = virtio_device_start_ioeventfd_impl; > vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl; > > +#ifdef CONFIG_VIRTIO_LEGACY > vdc->legacy_features |= VIRTIO_LEGACY_FEATURES; > +#endif > } > > bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev) > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 5e951675c60..d01fcb9fd1a 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -39,6 +39,7 @@ > #if !defined(CONFIG_USER_ONLY) > #include "hw/loader.h" > #include "hw/boards.h" > +#include CONFIG_DEVICES > #ifdef CONFIG_TCG > #include "hw/intc/armv7m_nvic.h" > #endif /* CONFIG_TCG */ > @@ -1130,6 +1131,7 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) > #endif > } > > +#ifdef CONFIG_VIRTIO_LEGACY > static bool arm_cpu_virtio_is_big_endian(CPUState *cs) > { > ARMCPU *cpu = ARM_CPU(cs); > @@ -1138,6 +1140,7 @@ static bool arm_cpu_virtio_is_big_endian(CPUState *cs) > cpu_synchronize_state(cs); > return arm_cpu_data_is_big_endian(env); > } > +#endif > > #ifdef CONFIG_TCG > bool arm_cpu_exec_halt(CPUState *cs) > @@ -2681,7 +2684,9 @@ static const struct SysemuCPUOps arm_sysemu_ops = { > .asidx_from_attrs = arm_asidx_from_attrs, > .write_elf32_note = arm_cpu_write_elf32_note, > .write_elf64_note = arm_cpu_write_elf64_note, > +#ifdef CONFIG_VIRTIO_LEGACY > .virtio_is_big_endian = arm_cpu_virtio_is_big_endian, > +#endif > .legacy_vmsd = &vmstate_arm_cpu, > }; > #endif > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > index b0973b6df95..4b6c347bda8 100644 > --- a/target/ppc/cpu_init.c > +++ b/target/ppc/cpu_init.c > @@ -50,6 +50,7 @@ > #include "hw/boards.h" > #include "hw/intc/intc.h" > #include "kvm_ppc.h" > +#include CONFIG_DEVICES > #endif > > #include "cpu_init.h" > @@ -7352,12 +7353,14 @@ static void ppc_cpu_reset_hold(Object *obj, ResetType type) > > #ifndef CONFIG_USER_ONLY > > +#ifdef CONFIG_VIRTIO_LEGACY > static bool ppc_cpu_is_big_endian(CPUState *cs) > { > cpu_synchronize_state(cs); > > return !FIELD_EX64(cpu_env(cs)->msr, MSR, LE); > } > +#endif > > static bool ppc_get_irq_stats(InterruptStatsProvider *obj, > uint64_t **irq_counts, unsigned int *nb_irqs) > @@ -7470,7 +7473,9 @@ static const struct SysemuCPUOps ppc_sysemu_ops = { > .get_phys_page_debug = ppc_cpu_get_phys_page_debug, > .write_elf32_note = ppc32_cpu_write_elf32_note, > .write_elf64_note = ppc64_cpu_write_elf64_note, > +#ifdef CONFIG_VIRTIO_LEGACY > .virtio_is_big_endian = ppc_cpu_is_big_endian, > +#endif > .legacy_vmsd = &vmstate_ppc_cpu, > }; > #endif > diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig > index 7648a2d68da..314185f0016 100644 > --- a/hw/virtio/Kconfig > +++ b/hw/virtio/Kconfig > @@ -1,6 +1,11 @@ > config VIRTIO > bool > > +config VIRTIO_LEGACY > + bool > + default y > + depends on VIRTIO > + > config VIRTIO_RNG > bool > default y If the goal is to condition virtio_legacy, to maybe deprecate it one day, then: Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On Fri, May 02, 2025 at 03:24:41PM +0200, Philippe Mathieu-Daudé wrote: > Legacy VirtIO devices don't have their endianness clearly defined. > QEMU infers it taking the endianness of the (target) binary, or, > when a target support switching endianness at runtime, taking the > endianness of the vCPU accessing the device. > > Devices modelling shouldn't really change depending on a property > of a CPU accessing it. > > For heterogeneous systems, it is simpler to break such dev <-> cpu > dependency, only allowing generic device models, with no knowledge > of CPU (or DMA controller) accesses. > > Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the > current default (enabled). > New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to > the VirtIO version 1 spec. IMHO that isn't acceptable. In order to be able to provide an upgrade path from the old binaries, we need the need the new binaries to be able to serve the same use cases & disabling virtio 0.9 support prevents that. I don't see a compelling technical reason why we can't support virtio 0.9 from this endian point. Yes may be more ugly/messy than we would like, but that's par for the course with QEMU emulating arbitrary device models. If the new binaries can't cope with messy devices then I think we are doing something wrong. With regards, Daniel
On Tue, May 06, 2025 at 09:04:50AM +0100, Daniel P. Berrangé wrote: > On Fri, May 02, 2025 at 03:24:41PM +0200, Philippe Mathieu-Daudé wrote: > > Legacy VirtIO devices don't have their endianness clearly defined. > > QEMU infers it taking the endianness of the (target) binary, or, > > when a target support switching endianness at runtime, taking the > > endianness of the vCPU accessing the device. > > > > Devices modelling shouldn't really change depending on a property > > of a CPU accessing it. > > > > For heterogeneous systems, it is simpler to break such dev <-> cpu > > dependency, only allowing generic device models, with no knowledge > > of CPU (or DMA controller) accesses. > > > > Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the > > current default (enabled). > > New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to > > the VirtIO version 1 spec. > > IMHO that isn't acceptable. In order to be able to provide an > upgrade path from the old binaries, we need the need the new > binaries to be able to serve the same use cases & disabling > virtio 0.9 support prevents that. I don't see a compelling > technical reason why we can't support virtio 0.9 from this > endian point. > > Yes may be more ugly/messy than we would like, but that's par > for the course with QEMU emulating arbitrary device models. > If the new binaries can't cope with messy devices then I think > we are doing something wrong. > > With regards, > Daniel To be more specific, having a kconfig knob modifying the device without regards for machine types, means it is no longer enough to inspect the command line to figure out the compatiblity. That's a problem.
On 6/5/25 10:12, Michael S. Tsirkin wrote: > On Tue, May 06, 2025 at 09:04:50AM +0100, Daniel P. Berrangé wrote: >> On Fri, May 02, 2025 at 03:24:41PM +0200, Philippe Mathieu-Daudé wrote: >>> Legacy VirtIO devices don't have their endianness clearly defined. >>> QEMU infers it taking the endianness of the (target) binary, or, >>> when a target support switching endianness at runtime, taking the >>> endianness of the vCPU accessing the device. >>> >>> Devices modelling shouldn't really change depending on a property >>> of a CPU accessing it. >>> >>> For heterogeneous systems, it is simpler to break such dev <-> cpu ^^^^^^^^^^^^^ >>> dependency, only allowing generic device models, with no knowledge >>> of CPU (or DMA controller) accesses. >>> >>> Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the >>> current default (enabled). >>> New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to >>> the VirtIO version 1 spec. >> >> IMHO that isn't acceptable. In order to be able to provide an >> upgrade path from the old binaries, we need the need the new >> binaries to be able to serve the same use cases & disabling >> virtio 0.9 support prevents that. This isn't for the single binary effort, there we are taking a lot of care to not introduce any change. This is for after it; once we have a single binary (one architecture run by an instance) we can start testing heterogeneous emulation (different architectures in the same instance). >> I don't see a compelling >> technical reason why we can't support virtio 0.9 from this >> endian point. VirtIO 0.9 needs knowledge of the vCPU architecture to get its endianness. So we need to somehow propagate that at creation time, guarantying which vCPU (or set of vCPUs) will access a virtio device. The use case I'd like to figure out is how should we model plugging a virtio 0.9 device in into a fully emulated ZynqMP machine, which has little-endian ARM cores and big endian MicroBlaze cores. Alex said this is unlikely to happen, and better is to ignore this case by not allowing virtio pre-1.0 devices in our future heterogeneous emulator. In this same thread Pierrick pointed me to the reference in the spec: '2.4.3 Legacy Interfaces: A Note on Virtqueue Endianness', "It is assumed that the host is already aware of the guest endian." I suppose this means a pre-1.0 virtio device expect to be used by a single guest OS, but it is not clear the guest OS as fixed endianness, because the code path checks vCPU endianness at runtime, so passing guest endianness as a property to pre-1.0 devices isn't really an option. Anyway I think 1/ I posted this too early, "speculating" as Pierrick noticed, and confuse the community w.r.t. "single binary" and 2/ I don' t understand legacy virtio and its endianness handling enough to figure a clever idea to keep using it heterogeneous setup, so I'll let this task to someone more familiar with that tech. >> Yes may be more ugly/messy than we would like, but that's par >> for the course with QEMU emulating arbitrary device models. >> If the new binaries can't cope with messy devices then I think >> we are doing something wrong. > > To be more specific, having a kconfig knob modifying the device > without regards for machine types, means it is no longer > enough to inspect the command line to figure out the > compatiblity. That's a problem. > OK. I won't pursue in this direction. I apologize for mentioning this topic again too early. Regards, Phil.
On Tue, May 06, 2025 at 10:55:34AM +0200, Philippe Mathieu-Daudé wrote: > On 6/5/25 10:12, Michael S. Tsirkin wrote: > > On Tue, May 06, 2025 at 09:04:50AM +0100, Daniel P. Berrangé wrote: > > > On Fri, May 02, 2025 at 03:24:41PM +0200, Philippe Mathieu-Daudé wrote: > > > > Legacy VirtIO devices don't have their endianness clearly defined. > > > > QEMU infers it taking the endianness of the (target) binary, or, > > > > when a target support switching endianness at runtime, taking the > > > > endianness of the vCPU accessing the device. > > > > > > > > Devices modelling shouldn't really change depending on a property > > > > of a CPU accessing it. > > > > > > > > For heterogeneous systems, it is simpler to break such dev <-> cpu > > ^^^^^^^^^^^^^ > > > > > dependency, only allowing generic device models, with no knowledge > > > > of CPU (or DMA controller) accesses. > > > > > > > > Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the > > > > current default (enabled). > > > > New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to > > > > the VirtIO version 1 spec. > > > > > > IMHO that isn't acceptable. In order to be able to provide an > > > upgrade path from the old binaries, we need the need the new > > > binaries to be able to serve the same use cases & disabling > > > virtio 0.9 support prevents that. > > This isn't for the single binary effort, there we are taking a > lot of care to not introduce any change. > > This is for after it; once we have a single binary (one architecture > run by an instance) we can start testing heterogeneous emulation > (different architectures in the same instance). > > > > I don't see a compelling > > > technical reason why we can't support virtio 0.9 from this > > > endian point. > > VirtIO 0.9 needs knowledge of the vCPU architecture to get its > endianness. So we need to somehow propagate that at creation > time, guarantying which vCPU (or set of vCPUs) will access a > virtio device. > > The use case I'd like to figure out is how should we model > plugging a virtio 0.9 device in into a fully emulated > ZynqMP machine, which has little-endian ARM cores and big > endian MicroBlaze cores. > > Alex said this is unlikely to happen, and better is to > ignore this case by not allowing virtio pre-1.0 devices in > our future heterogeneous emulator. Indeed. I just do not think this can be done with a kconfig knob, it's a machine property. > In this same thread Pierrick pointed me to the reference in > the spec: '2.4.3 Legacy Interfaces: A Note on Virtqueue Endianness', > "It is assumed that the host is already aware of the guest endian." > > I suppose this means a pre-1.0 virtio device expect to be used by > a single guest OS, but it is not clear the guest OS as fixed > endianness, because the code path checks vCPU endianness at > runtime, so passing guest endianness as a property to pre-1.0 > devices isn't really an option. > > Anyway I think 1/ I posted this too early, "speculating" as Pierrick > noticed, and confuse the community w.r.t. "single binary" and > 2/ I don' t understand legacy virtio and its endianness handling > enough to figure a clever idea to keep using it heterogeneous > setup, so I'll let this task to someone more familiar with that tech. > > > > Yes may be more ugly/messy than we would like, but that's par > > > for the course with QEMU emulating arbitrary device models. > > > If the new binaries can't cope with messy devices then I think > > > we are doing something wrong. > > > > > To be more specific, having a kconfig knob modifying the device > > without regards for machine types, means it is no longer > > enough to inspect the command line to figure out the > > compatiblity. That's a problem. > > > > OK. I won't pursue in this direction. I apologize for mentioning > this topic again too early. > > Regards, > > Phil.
On 5/6/25 1:55 AM, Philippe Mathieu-Daudé wrote: > On 6/5/25 10:12, Michael S. Tsirkin wrote: >> On Tue, May 06, 2025 at 09:04:50AM +0100, Daniel P. Berrangé wrote: >>> On Fri, May 02, 2025 at 03:24:41PM +0200, Philippe Mathieu-Daudé wrote: >>>> Legacy VirtIO devices don't have their endianness clearly defined. >>>> QEMU infers it taking the endianness of the (target) binary, or, >>>> when a target support switching endianness at runtime, taking the >>>> endianness of the vCPU accessing the device. >>>> >>>> Devices modelling shouldn't really change depending on a property >>>> of a CPU accessing it. >>>> >>>> For heterogeneous systems, it is simpler to break such dev <-> cpu > > ^^^^^^^^^^^^^ > >>>> dependency, only allowing generic device models, with no knowledge >>>> of CPU (or DMA controller) accesses. >>>> >>>> Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the >>>> current default (enabled). >>>> New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to >>>> the VirtIO version 1 spec. >>> >>> IMHO that isn't acceptable. In order to be able to provide an >>> upgrade path from the old binaries, we need the need the new >>> binaries to be able to serve the same use cases & disabling >>> virtio 0.9 support prevents that. > > This isn't for the single binary effort, there we are taking a > lot of care to not introduce any change. > > This is for after it; once we have a single binary (one architecture > run by an instance) we can start testing heterogeneous emulation > (different architectures in the same instance). > >>> I don't see a compelling >>> technical reason why we can't support virtio 0.9 from this >>> endian point. > > VirtIO 0.9 needs knowledge of the vCPU architecture to get its > endianness. So we need to somehow propagate that at creation > time, guarantying which vCPU (or set of vCPUs) will access a > virtio device. > > The use case I'd like to figure out is how should we model > plugging a virtio 0.9 device in into a fully emulated > ZynqMP machine, which has little-endian ARM cores and big > endian MicroBlaze cores. > Virtio devices are not the only one who will need to be extended to support such a scenario. What happens when a disk or network card is added to the machine? Should it be shared? Should it be mapped only for one cpu address space? Should address spaces be mixed or independent? Per cluster, per cpu, per architure? Having a concrete prototype will allow us to answer to those questions, and many others, and find solutions for the situations we meet. > Alex said this is unlikely to happen, and better is to > ignore this case by not allowing virtio pre-1.0 devices in > our future heterogeneous emulator. > > In this same thread Pierrick pointed me to the reference in > the spec: '2.4.3 Legacy Interfaces: A Note on Virtqueue Endianness', > "It is assumed that the host is already aware of the guest endian." > > I suppose this means a pre-1.0 virtio device expect to be used by > a single guest OS, but it is not clear the guest OS as fixed > endianness, because the code path checks vCPU endianness at > runtime, so passing guest endianness as a property to pre-1.0 > devices isn't really an option. > > Anyway I think 1/ I posted this too early, "speculating" as Pierrick > noticed, and confuse the community w.r.t. "single binary" and > 2/ I don' t understand legacy virtio and its endianness handling > enough to figure a clever idea to keep using it heterogeneous > setup, so I'll let this task to someone more familiar with that tech. > An important mantra will be to keep that compatible with existing command line (potentially extending it to support those new use cases, in both the existing *and* the new binary). The last thing we want to introduce is "yet another QEMU". >>> Yes may be more ugly/messy than we would like, but that's par >>> for the course with QEMU emulating arbitrary device models. >>> If the new binaries can't cope with messy devices then I think >>> we are doing something wrong. > >> >> To be more specific, having a kconfig knob modifying the device >> without regards for machine types, means it is no longer >> enough to inspect the command line to figure out the >> compatiblity. That's a problem. >> > > OK. I won't pursue in this direction. I apologize for mentioning > this topic again too early. > > Regards, > > Phil.
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h index 07aae69042a..b5b471711a6 100644 --- a/include/hw/virtio/virtio-access.h +++ b/include/hw/virtio/virtio-access.h @@ -20,7 +20,10 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-bus.h" -#if defined(TARGET_PPC64) || defined(TARGET_ARM) +#include CONFIG_DEVICES + +#if defined(CONFIG_VIRTIO_LEGACY) && \ + (defined(TARGET_PPC64) || defined(TARGET_ARM)) #define LEGACY_VIRTIO_IS_BIENDIAN 1 #endif diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 480c2e50365..659ab3cb969 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -47,6 +47,8 @@ #include "standard-headers/linux/virtio_mem.h" #include "standard-headers/linux/virtio_vsock.h" +#include CONFIG_DEVICES + /* * Maximum size of virtio device config space */ @@ -3502,6 +3504,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t device_id, size_t config_size) bool virtio_legacy_allowed(VirtIODevice *vdev) { switch (vdev->device_id) { +#ifdef CONFIG_VIRTIO_LEGACY case VIRTIO_ID_NET: case VIRTIO_ID_BLOCK: case VIRTIO_ID_CONSOLE: @@ -3513,6 +3516,7 @@ bool virtio_legacy_allowed(VirtIODevice *vdev) case VIRTIO_ID_RPROC_SERIAL: case VIRTIO_ID_CAIF: return true; +#endif default: return false; } @@ -4014,8 +4018,10 @@ static const Property virtio_properties[] = { DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features), DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true), DEFINE_PROP_BOOL("use-disabled-flag", VirtIODevice, use_disabled_flag, true), +#ifdef CONFIG_VIRTIO_LEGACY DEFINE_PROP_BOOL("x-disable-legacy-check", VirtIODevice, disable_legacy_check, false), +#endif }; static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev) @@ -4151,7 +4157,9 @@ static void virtio_device_class_init(ObjectClass *klass, const void *data) vdc->start_ioeventfd = virtio_device_start_ioeventfd_impl; vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl; +#ifdef CONFIG_VIRTIO_LEGACY vdc->legacy_features |= VIRTIO_LEGACY_FEATURES; +#endif } bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 5e951675c60..d01fcb9fd1a 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -39,6 +39,7 @@ #if !defined(CONFIG_USER_ONLY) #include "hw/loader.h" #include "hw/boards.h" +#include CONFIG_DEVICES #ifdef CONFIG_TCG #include "hw/intc/armv7m_nvic.h" #endif /* CONFIG_TCG */ @@ -1130,6 +1131,7 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int level) #endif } +#ifdef CONFIG_VIRTIO_LEGACY static bool arm_cpu_virtio_is_big_endian(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); @@ -1138,6 +1140,7 @@ static bool arm_cpu_virtio_is_big_endian(CPUState *cs) cpu_synchronize_state(cs); return arm_cpu_data_is_big_endian(env); } +#endif #ifdef CONFIG_TCG bool arm_cpu_exec_halt(CPUState *cs) @@ -2681,7 +2684,9 @@ static const struct SysemuCPUOps arm_sysemu_ops = { .asidx_from_attrs = arm_asidx_from_attrs, .write_elf32_note = arm_cpu_write_elf32_note, .write_elf64_note = arm_cpu_write_elf64_note, +#ifdef CONFIG_VIRTIO_LEGACY .virtio_is_big_endian = arm_cpu_virtio_is_big_endian, +#endif .legacy_vmsd = &vmstate_arm_cpu, }; #endif diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index b0973b6df95..4b6c347bda8 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -50,6 +50,7 @@ #include "hw/boards.h" #include "hw/intc/intc.h" #include "kvm_ppc.h" +#include CONFIG_DEVICES #endif #include "cpu_init.h" @@ -7352,12 +7353,14 @@ static void ppc_cpu_reset_hold(Object *obj, ResetType type) #ifndef CONFIG_USER_ONLY +#ifdef CONFIG_VIRTIO_LEGACY static bool ppc_cpu_is_big_endian(CPUState *cs) { cpu_synchronize_state(cs); return !FIELD_EX64(cpu_env(cs)->msr, MSR, LE); } +#endif static bool ppc_get_irq_stats(InterruptStatsProvider *obj, uint64_t **irq_counts, unsigned int *nb_irqs) @@ -7470,7 +7473,9 @@ static const struct SysemuCPUOps ppc_sysemu_ops = { .get_phys_page_debug = ppc_cpu_get_phys_page_debug, .write_elf32_note = ppc32_cpu_write_elf32_note, .write_elf64_note = ppc64_cpu_write_elf64_note, +#ifdef CONFIG_VIRTIO_LEGACY .virtio_is_big_endian = ppc_cpu_is_big_endian, +#endif .legacy_vmsd = &vmstate_ppc_cpu, }; #endif diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig index 7648a2d68da..314185f0016 100644 --- a/hw/virtio/Kconfig +++ b/hw/virtio/Kconfig @@ -1,6 +1,11 @@ config VIRTIO bool +config VIRTIO_LEGACY + bool + default y + depends on VIRTIO + config VIRTIO_RNG bool default y
Legacy VirtIO devices don't have their endianness clearly defined. QEMU infers it taking the endianness of the (target) binary, or, when a target support switching endianness at runtime, taking the endianness of the vCPU accessing the device. Devices modelling shouldn't really change depending on a property of a CPU accessing it. For heterogeneous systems, it is simpler to break such dev <-> cpu dependency, only allowing generic device models, with no knowledge of CPU (or DMA controller) accesses. Therefore we introduce the VIRTIO_LEGACY Kconfig key. We keep the current default (enabled). New binaries can set CONFIG_VIRTIO_LEGACY=n to restrict models to the VirtIO version 1 spec. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/virtio/virtio-access.h | 5 ++++- hw/virtio/virtio.c | 8 ++++++++ target/arm/cpu.c | 5 +++++ target/ppc/cpu_init.c | 5 +++++ hw/virtio/Kconfig | 5 +++++ 5 files changed, 27 insertions(+), 1 deletion(-)