Re: [PATCH v2 02/11] acpi: Define ACPI IO registers for PVH guests

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 11/09/2016 09:59 AM, Andrew Cooper wrote:
>
>> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
>> index 2e5809b..e3fa704 100644
>> --- a/xen/include/public/hvm/ioreq.h
>> +++ b/xen/include/public/hvm/ioreq.h
>> @@ -24,6 +24,8 @@
>>  #ifndef _IOREQ_H_
>>  #define _IOREQ_H_
>>  
>> +#include "hvm_info_table.h" /* HVM_MAX_VCPUS */
>> +
>>  #define IOREQ_READ      1
>>  #define IOREQ_WRITE     0
>>  
>> @@ -124,6 +126,17 @@ typedef struct buffered_iopage buffered_iopage_t;
>>  #define ACPI_GPE0_BLK_ADDRESS        ACPI_GPE0_BLK_ADDRESS_V0
>>  #define ACPI_GPE0_BLK_LEN            ACPI_GPE0_BLK_LEN_V0
>>  
>> +#define ACPI_PM1A_EVT_BLK_LEN        0x04
>> +#define ACPI_PM1A_CNT_BLK_LEN        0x02
>> +#define ACPI_PM_TMR_BLK_LEN          0x04
>> +
>> +/* Location of online VCPU bitmap. */
>> +#define ACPI_CPU_MAP                 0xaf00
>> +#define ACPI_CPU_MAP_LEN             ((HVM_MAX_VCPUS / 8) + \
>> +                                      ((HVM_MAX_VCPUS & 7) ? 1 : 0))
>> +#if ACPI_CPU_MAP + ACPI_CPU_MAP_LEN >= ACPI_GPE0_BLK_ADDRESS_V1
>> +#error "ACPI_CPU_MAP is too big"
>> +#endif
> Why is this in ioreq.h?  It has nothing to do with ioreq's.
>
> The current ACPI bits in here are to do with the qemu ACPI interface,
> not the Xen ACPI interface.
>
> Also, please can we avoid hard-coding the location of the map in the
> hypervisor ABI.  These constants make it impossible to ever extend the
> number of HVM vcpus at a future date.

The first three logically belong here because corresponding blocks'
addresses are defined right above.

ACPI_CPU_MAP has to be seen by both the toolstack (libacpi) and the
hypervisor (and qemu as well, although it is defined as
PIIX4_CPU_HOTPLUG_IO_BASE).

Where do you think it should go then?

-boris



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