| Message ID | 1480433602-13290-8-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote: > +void hvm_acpi_init(struct domain *d) > +{ > + if ( has_acpi_dm_ff(d) ) > + return; > + > + register_portio_handler(d, XEN_ACPI_CPU_MAP, > + XEN_ACPI_CPU_MAP_LEN, acpi_guest_access); > + register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1, > + sizeof (d->arch.hvm_domain.acpi.gpe0_sts) + > + sizeof (d->arch.hvm_domain.acpi.gpe0_en), Keyword or not, we don't put spaces between sizeof and the opening parenthesis. > + acpi_guest_access); > + register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1, > + sizeof (d->arch.hvm_domain.acpi.pm1a_sts) + > + sizeof (d->arch.hvm_domain.acpi.pm1a_en), > + acpi_guest_access); Does it really result in most legible / maintainable code (in subsequent patches) if all three use the same handler? > --- a/xen/include/public/arch-x86/hvm/save.h > +++ b/xen/include/public/arch-x86/hvm/save.h > @@ -532,6 +532,8 @@ struct hvm_hw_acpi { > uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */ > uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */ > uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */ > + uint16_t gpe0_sts; > + uint16_t gpe0_en; > }; Don't you need to create compat handling for the case where a smaller struct arrives during migration? Or do we zero extend structures nowadays without extra code being needed (assuming zero extension is what we want in that case in the first place)? Also the same remark regarding this not being __XEN_TOOLS__ guarded applies here. Jan
On 12/01/2016 11:32 AM, Jan Beulich wrote: >>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote: >> +void hvm_acpi_init(struct domain *d) >> +{ >> + if ( has_acpi_dm_ff(d) ) >> + return; >> + >> + register_portio_handler(d, XEN_ACPI_CPU_MAP, >> + XEN_ACPI_CPU_MAP_LEN, acpi_guest_access); >> + register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1, >> + sizeof (d->arch.hvm_domain.acpi.gpe0_sts) + >> + sizeof (d->arch.hvm_domain.acpi.gpe0_en), > Keyword or not, we don't put spaces between sizeof and the > opening parenthesis. > >> + acpi_guest_access); >> + register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1, >> + sizeof (d->arch.hvm_domain.acpi.pm1a_sts) + >> + sizeof (d->arch.hvm_domain.acpi.pm1a_en), >> + acpi_guest_access); > Does it really result in most legible / maintainable code (in > subsequent patches) if all three use the same handler? I can split them into 2 --- one for pm1a/gpe0 and the other for CPU map. They are indeed handled differently. > >> --- a/xen/include/public/arch-x86/hvm/save.h >> +++ b/xen/include/public/arch-x86/hvm/save.h >> @@ -532,6 +532,8 @@ struct hvm_hw_acpi { >> uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */ >> uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */ >> uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */ >> + uint16_t gpe0_sts; >> + uint16_t gpe0_en; >> }; > Don't you need to create compat handling for the case where a > smaller struct arrives during migration? Or do we zero extend > structures nowadays without extra code being needed (assuming > zero extension is what we want in that case in the first place)? I thought that the fact that new fields are added at the end would make this safe. And I assumed we will always read into a newly-allocated hvm_domain, so those registers would be zero. But being more explicit about this is probably better, I'll add DECLARE_HVM_SAVE_TYPE_COMPAT. Andrew, will this interfere with your hypervisor migration v2 plan? -boris
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c index 7d42eaf..a1f7fd2 100644 --- a/xen/arch/x86/hvm/acpi.c +++ b/xen/arch/x86/hvm/acpi.c @@ -6,6 +6,7 @@ #include <xen/lib.h> #include <xen/sched.h> +#include <public/arch-x86/xen.h> int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas, XEN_GUEST_HANDLE_PARAM(uint8) arg) @@ -13,6 +14,29 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas, return -ENOSYS; } +static int acpi_guest_access(int dir, unsigned int port, + unsigned int bytes, uint32_t *val) +{ + return X86EMUL_UNHANDLEABLE; +} + +void hvm_acpi_init(struct domain *d) +{ + if ( has_acpi_dm_ff(d) ) + return; + + register_portio_handler(d, XEN_ACPI_CPU_MAP, + XEN_ACPI_CPU_MAP_LEN, acpi_guest_access); + register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1, + sizeof (d->arch.hvm_domain.acpi.gpe0_sts) + + sizeof (d->arch.hvm_domain.acpi.gpe0_en), + acpi_guest_access); + register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1, + sizeof (d->arch.hvm_domain.acpi.pm1a_sts) + + sizeof (d->arch.hvm_domain.acpi.pm1a_en), + acpi_guest_access); +} + /* * Local variables: * mode: C diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 25dc759..9eb518f 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -647,6 +647,8 @@ int hvm_domain_initialise(struct domain *d) hvm_ioreq_init(d); + hvm_acpi_init(d); + if ( is_pvh_domain(d) ) { register_portio_handler(d, 0, 0x10003, handle_pvh_io); diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index f6a40eb..9509b9e 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -425,6 +425,8 @@ struct arch_domain #define has_vvga(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_VGA)) #define has_viommu(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_IOMMU)) #define has_vpit(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT)) +#define has_acpi_dm_ff(d) (!!((d)->arch.emulation_flags & \ + XEN_X86_EMU_ACPI_DM_FF)) #define has_arch_pdevs(d) (!list_empty(&(d)->arch.pdev_list)) diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h index c5cd86c..7ca3b40 100644 --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -158,6 +158,7 @@ struct hvm_domain { #define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled) +void hvm_acpi_init(struct domain *d); int hvm_acpi_domctl_access(struct domain *currd, uint8_t rw, gas_t *gas, XEN_GUEST_HANDLE_PARAM(uint8) arg); diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h index 3997487..682a738 100644 --- a/xen/include/public/arch-x86/hvm/save.h +++ b/xen/include/public/arch-x86/hvm/save.h @@ -532,6 +532,8 @@ struct hvm_hw_acpi { uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */ uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */ uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */ + uint16_t gpe0_sts; + uint16_t gpe0_en; }; DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi); diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h index 0e3a3df..ec5499f 100644 --- a/xen/include/public/arch-x86/xen.h +++ b/xen/include/public/arch-x86/xen.h @@ -283,12 +283,14 @@ struct xen_arch_domainconfig { #define XEN_X86_EMU_IOMMU (1U<<_XEN_X86_EMU_IOMMU) #define _XEN_X86_EMU_PIT 8 #define XEN_X86_EMU_PIT (1U<<_XEN_X86_EMU_PIT) +#define _XEN_X86_EMU_ACPI_DM_FF 9 +#define XEN_X86_EMU_ACPI_DM_FF (1U<<_XEN_X86_EMU_ACPI_DM_FF) #define XEN_X86_EMU_ALL (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET | \ XEN_X86_EMU_PM | XEN_X86_EMU_RTC | \ XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC | \ XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU | \ - XEN_X86_EMU_PIT) + XEN_X86_EMU_PIT | XEN_X86_EMU_ACPI_DM_FF) uint32_t emulation_flags; };
PVH guests will have ACPI accesses emulated by the hypervisor as opposed to QEMU (as is the case for HVM guests). This patch installs handler for accesses to PM1A, GPE0 (which is added to struct hvm_hw_acpi) and VCPU map. Logic for the handler will be provided by a later patch. Whether or not the handler needs to be installed is decided based on the value of XEN_X86_EMU_ACPI_FF flag which indicates whether emulation is implemented in QEMU. Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- Changes in v4: * Moved handler registration from ioreq.c to acpi.c * Renamed XEN_X86_EMU_ACPI_FF to XEN_X86_EMU_ACPI_DM_FF (with a corresponding change to has_* macro) xen/arch/x86/hvm/acpi.c | 24 ++++++++++++++++++++++++ xen/arch/x86/hvm/hvm.c | 2 ++ xen/include/asm-x86/domain.h | 2 ++ xen/include/asm-x86/hvm/domain.h | 1 + xen/include/public/arch-x86/hvm/save.h | 2 ++ xen/include/public/arch-x86/xen.h | 4 +++- 6 files changed, 34 insertions(+), 1 deletion(-)