Re: [Xen-devel] [PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@xxxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel