acpi_piix4: save gpe and pci hotplug slot status
PATCH RESEND 1/1 acpi_piix4: save gpe and pci hotplug slot status
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
---
hw/acpi_piix4.c | 29 ++++++++++++++++++++++++++++- 1 files changed, 28 insertions(+), 1 deletions(-)
hw/acpi_piix4.c +28 -1
@@ -283,9 +283,33 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)return 0 ; } +static const VMStateDescription vmstate_gpe = { + .name ="gpe" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { + VMSTATE_UINT16(sts, struct gpe_regs), + VMSTATE_UINT16(en, struct gpe_regs), +VMSTATE_END_OF_LIST () + } +}; + +static const VMStateDescription vmstate_pci_status = { + .name ="pci_status" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { + VMSTATE_UINT32(up, struct pci_status), + VMSTATE_UINT32(down, struct pci_status), +VMSTATE_END_OF_LIST () + } +}; +static const VMStateDescription vmstate_acpi = { .name ="piix4_pm" , - .version_id =1 , + .version_id =2 , .minimum_version_id =1 , .minimum_version_id_old =1 , .post_load = vmstate_acpi_post_load,
@@ -297,6 +321,9 @@ static const VMStateDescription vmstate_acpi = {VMSTATE_STRUCT (apm, PIIX4PMState,0 , vmstate_apm, APMState),VMSTATE_TIMER (tmr_timer, PIIX4PMState),VMSTATE_INT64 (tmr_overflow_time, PIIX4PMState), + VMSTATE_STRUCT(gpe, PIIX4PMState, 2, vmstate_gpe, struct gpe_regs), +VMSTATE_STRUCT (pci0_status, PIIX4PMState,2 , vmstate_pci_status, + struct pci_status), VMSTATE_END_OF_LIST() } };
Anthony Liguori
AppliedQuoted 6 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Signed-off-by Alex Williamson<[email protected]>
Applied. Thanks.
Regards,
Anthony Liguori
Quoted 58 lines
---
hw/acpi_piix4.c | 29 ++++++++++++++++++++++++++++- 1 files changed, 28 insertions(+), 1 deletions(-)hw/acpi_piix4.c +28
-1@@ -283,9 +283,33 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)return 0 ; } +static const VMStateDescription vmstate_gpe = { + .name ="gpe" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { + VMSTATE_UINT16(sts, struct gpe_regs), + VMSTATE_UINT16(en, struct gpe_regs), +VMSTATE_END_OF_LIST () + } +}; + +static const VMStateDescription vmstate_pci_status = { + .name ="pci_status" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { + VMSTATE_UINT32(up, struct pci_status), + VMSTATE_UINT32(down, struct pci_status), +VMSTATE_END_OF_LIST () + } +}; +static const VMStateDescription vmstate_acpi = { .name ="piix4_pm" , - .version_id =1 , + .version_id =2 , .minimum_version_id =1 , .minimum_version_id_old =1 , .post_load = vmstate_acpi_post_load,@@ -297,6 +321,9 @@ static const VMStateDescription vmstate_acpi = {VMSTATE_STRUCT (apm, PIIX4PMState,0 , vmstate_apm, APMState),VMSTATE_TIMER (tmr_timer, PIIX4PMState),VMSTATE_INT64 (tmr_overflow_time, PIIX4PMState), + VMSTATE_STRUCT(gpe, PIIX4PMState, 2, vmstate_gpe, struct gpe_regs), +VMSTATE_STRUCT (pci0_status, PIIX4PMState,2 , vmstate_pci_status, + struct pci_status), VMSTATE_END_OF_LIST() } };
Quoted 9 lines · Anthony Liguori
Quoted 6 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Is this stable material?
Alex
Quoted 5 lines · Anthony Liguori
Quoted 4 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
This way it could also be backported to stable.
Later, Juan.
Quoted 13 lines · Juan Quintela
Quoted 12 lines · Anthony Liguori
Quoted 6 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Quoted 7 lines
Quoted 1 line
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
This way it could also be backported to stable.
The slots aren't really the issue, they were mostly for completeness. The key is gpe.en, which is likely always going to be all 1s for an ACPI aware OS. So if we test != 0, we're going to need that subsection in 99% of the cases. Maybe we can assume gpe.en is all set on the target, but I don't really look forward to finding out the ways that might break. Thanks,
Alex
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Juan Quintela
Quoted 5 lines · Anthony Liguori
Quoted 4 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
Quoted 8 lines
Quoted 1 line
This way it could also be backported to stable.
The slots aren't really the issue, they were mostly for completeness. The key is gpe.en, which is likely always going to be all 1s for an ACPI aware OS. So if we test != 0, we're going to need that subsection in 99% of the cases. Maybe we can assume gpe.en is all set on the target, but I don't really look forward to finding out the ways that might break. Thanks,
We have never sent it before. That means that the default value (for whatever value is it) should be working quite well.
But it is not my call.
Later, Juan.
Quoted 34 lines · Juan Quintela
Quoted 23 lines · Alex Williamson
Quoted 21 lines · Juan Quintela
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Quoted 9 lines
Quoted 3 lines
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
Quoted 9 lines
Quoted 2 lines
This way it could also be backported to stable.
The slots aren't really the issue, they were mostly for completeness. The key is gpe.en, which is likely always going to be all 1s for an ACPI aware OS. So if we test != 0, we're going to need that subsection in 99% of the cases. Maybe we can assume gpe.en is all set on the target, but I don't really look forward to finding out the ways that might break. Thanks,
Quoted 5 lines
We have never sent it before. That means that the default value (for whatever value is it) should be working quite well.
But it is not my call.
This sounds like a case where we're sending broken state so we need to bump the version number. Subsections don't eliminate the need to bump version numbers when we fundamentally screw up.
Regards,
Anthony Liguori
Quoted 2 lines
Later, Juan.
Quoted 30 lines · Juan Quintela
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Juan Quintela
Quoted 5 lines · Anthony Liguori
Quoted 4 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
Quoted 8 lines
Quoted 1 line
This way it could also be backported to stable.
The slots aren't really the issue, they were mostly for completeness. The key is gpe.en, which is likely always going to be all 1s for an ACPI aware OS. So if we test != 0, we're going to need that subsection in 99% of the cases. Maybe we can assume gpe.en is all set on the target, but I don't really look forward to finding out the ways that might break. Thanks,
Quoted 2 lines
We have never sent it before. That means that the default value (for whatever value is it) should be working quite well.
The gpe.en bits back an ioport range that's poked by the guest via ACPI. So the guest is the one telling ACPI "I enable you to send me this gpe". When a hotplug event happens, we figure out which gpe line it would toggle, set the matching gpe.sts bit, then if the OS has told us to enable that bit, we send an interrupt. The OS then masks gpe.en, checks gpe.sts, preforms actions and clears the respective gpe.sts bits, then re-enables gpe.en. So, qemu never sets the bits, but if we decide to clear (or set) them, we're interfering with how the OS expects them to work. I think we're screwed, we need to bump the rev.
Alex
Quoted 34 lines · Alex Williamson
Quoted 30 lines · Juan Quintela
Quoted 20 lines · Alex Williamson
Quoted 19 lines · Juan Quintela
Quoted 5 lines · Anthony Liguori
Quoted 4 lines · Alex Williamson
PCI hotplug currently doesn't work after a migration because we don't migrate the enable bits of the GPE state. Pull hotplug structs into vmstate.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Alex Williamson<[email protected]>Applied. Thanks.
Regards,
Anthony Liguori
I think this is better implemented as a subsection. We didin't need this until hotplug arrived, I think that checking if any up/down are != 0 and then send it as subsections is a best way to do it.
Quoted 8 lines
Quoted 1 line
This way it could also be backported to stable.
The slots aren't really the issue, they were mostly for completeness. The key is gpe.en, which is likely always going to be all 1s for an ACPI aware OS. So if we test != 0, we're going to need that subsection in 99% of the cases. Maybe we can assume gpe.en is all set on the target, but I don't really look forward to finding out the ways that might break. Thanks,
Quoted 2 lines
We have never sent it before. That means that the default value (for whatever value is it) should be working quite well.
Quoted 9 lines
The gpe.en bits back an ioport range that's poked by the guest via ACPI. So the guest is the one telling ACPI "I enable you to send me this gpe". When a hotplug event happens, we figure out which gpe line it would toggle, set the matching gpe.sts bit, then if the OS has told us to enable that bit, we send an interrupt. The OS then masks gpe.en, checks gpe.sts, preforms actions and clears the respective gpe.sts bits, then re-enables gpe.en. So, qemu never sets the bits, but if we decide to clear (or set) them, we're interfering with how the OS expects them to work. I think we're screwed, we need to bump the rev.
Sniff. I tried. Cross version migration is such a nice thing :(
Later, Juan.