[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] hw/acpi/vmgenid: prevent device realization
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] hw/acpi/vmgenid: prevent device realization on pre-2.9 machine types |
Date: |
Mon, 20 Mar 2017 16:08:06 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 03/20/17 15:39, Paolo Bonzini wrote:
>
>
> On 20/03/2017 15:19, Michael S. Tsirkin wrote:
>> On Mon, Mar 20, 2017 at 12:59:50PM +0100, Laszlo Ersek wrote:
>>> The WRITE_POINTER linker/loader command that underlies VMGENID depends on
>>> commit baf2d5bfbac0 ("fw-cfg: support writeable blobs", 2017-01-12). That
>>> commit is not available in 2.8.
>>>
>>> Cc: "Michael S. Tsirkin" <address@hidden>
>>> Cc: Ben Warren <address@hidden>
>>> Cc: Igor Mammedov <address@hidden>
>>> Signed-off-by: Laszlo Ersek <address@hidden>
>>
>> I don't understand why do we want to add code to break this
>> intentionally. What issue does this fix?
>
> Agreed. If vmgenid can work with old machine types and new QEMU, it
> should. For example, when using libvirt machine types identify the
> machine type where the VM was _created_ (unless someone changes it), not
> necessarily the minimal QEMU version that can run it.
Yes, I understand that.
The problem is this:
(1) machine types up to and including 2.4 do not enable DMA for fw_cfg.
Without DMA, the WRITE_POINTER linker/loader commands have no chance of
working in the firmware.
(2) ... the other part of the problem was actually in my thinking, sorry
about that. I forgot that that "-device vmgenid" itself is not available
in QEMU releases prior to 2.9. So we shouldn't worry about the case
where write support is missing from fw_cfg, although DMA is available.
In this patch I tried to address (non-)problem (2), thereby covering
(real) problem (1). I now see that (2) doesn't actually exist. (1) does
have to be addressed however.
If you agree, I'll respin this patch.
Thanks
Laszlo
>
> Paolo
>
>>> ---
>>> include/hw/acpi/vmgenid.h | 1 +
>>> include/hw/compat.h | 4 ++++
>>> hw/acpi/vmgenid.c | 14 ++++++++++++++
>>> 3 files changed, 19 insertions(+)
>>>
>>> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
>>> index db7fa0e63303..8578476baebe 100644
>>> --- a/include/hw/acpi/vmgenid.h
>>> +++ b/include/hw/acpi/vmgenid.h
>>> @@ -21,6 +21,7 @@ typedef struct VmGenIdState {
>>> DeviceClass parent_obj;
>>> QemuUUID guid; /* The 128-bit GUID seen by the guest */
>>> uint8_t vmgenid_addr_le[8]; /* Address of the GUID (little-endian) */
>>> + bool write_pointer_available;
>>> } VmGenIdState;
>>>
>>> static inline Object *find_vmgenid_dev(void)
>>> diff --git a/include/hw/compat.h b/include/hw/compat.h
>>> index fc8c3e060007..c9caa8cd1bdd 100644
>>> --- a/include/hw/compat.h
>>> +++ b/include/hw/compat.h
>>> @@ -42,6 +42,10 @@
>>> .driver = "isa-cirrus-vga",\
>>> .property = "vgamem_mb",\
>>> .value = "8",\
>>> + },{\
>>> + .driver = "vmgenid",\
>>> + .property = "x-write-pointer-available",\
>>> + .value = "off",\
>>> },
>>>
>>> #define HW_COMPAT_2_7 \
>>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>>> index 7a3ad17d66ef..c3ddcc8e7cb0 100644
>>> --- a/hw/acpi/vmgenid.c
>>> +++ b/hw/acpi/vmgenid.c
>>> @@ -205,9 +205,22 @@ static void vmgenid_handle_reset(void *opaque)
>>> memset(vms->vmgenid_addr_le, 0, ARRAY_SIZE(vms->vmgenid_addr_le));
>>> }
>>>
>>> +static Property vmgenid_properties[] = {
>>> + DEFINE_PROP_BOOL("x-write-pointer-available", VmGenIdState,
>>> + write_pointer_available, true),
>>> + DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> static void vmgenid_realize(DeviceState *dev, Error **errp)
>>> {
>>> VmGenIdState *vms = VMGENID(dev);
>>> +
>>> + if (!vms->write_pointer_available) {
>>> + error_setg(errp, "%s requires DMA write support in fw_cfg, "
>>> + "which this machine type does not provide",
>>> VMGENID_DEVICE);
>>> + return;
>>> + }
>>> +
>>> qemu_register_reset(vmgenid_handle_reset, vms);
>>> }
>>>
>>> @@ -218,6 +231,7 @@ static void vmgenid_device_class_init(ObjectClass
>>> *klass, void *data)
>>> dc->vmsd = &vmstate_vmgenid;
>>> dc->realize = vmgenid_realize;
>>> dc->hotpluggable = false;
>>> + dc->props = vmgenid_properties;
>>>
>>> object_class_property_add_str(klass, VMGENID_GUID, NULL,
>>> vmgenid_set_guid, NULL);
>>> --
>>> 2.9.3
>>>
>>
[Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20