Re: [Xen-devel] [PATCH RFC 13/20] acpi/hvmloader: Add stdio.h, string.h and x86.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.06.16 at 17:08, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 06/06/2016 07:31 AM, Jan Beulich wrote:
>>>>> On 06.04.16 at 03:25, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> --- /dev/null
>>> +++ b/tools/firmware/hvmloader/acpi/x86.h
>>> @@ -0,0 +1,14 @@
>>> +#ifndef __ACPI_X86_H__
>>> +#define __ACPI_X86_H__
>>> +
>>> +#define IOAPIC_BASE_ADDRESS 0xfec00000
>>> +#define IOAPIC_ID           0x01
>>> +#define IOAPIC_VERSION      0x11
>>> +
>>> +#define LAPIC_BASE_ADDRESS  0xfee00000
>>> +#define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
>>> +
>>> +#define PCI_ISA_DEVFN       0x08    /* dev 1, fn 0 */
>>> +#define PCI_ISA_IRQ_MASK    0x0c20U /* ISA IRQs 5,10,11 are PCI connected 
>>> */
>> Only the latter of those last two is used by build.c. Please limit what
>> you move to the very minimum. I.e. IOAPIC_VERSION shouldn't move
>> either imo (and I notice there's a disconnect between it and the
>> hypervisor's VIOAPIC_VERSION_ID, but that's a pre-existing issue).
> 
> You don't think logically all these should be in the same file?

Not really, no. IOAPIC_VERSION, as said, should go away.
Meanwhile I already have a patch for that (at once dropping
IOAPIC_BASE_ADDRESS too), albeit that conflicts with what
you're doing.

And the two PCI_ISA_ ones seem to belong together solely
because of their name. The IRQ mask is a simple number,
unrelated to any specific PCI device.

Jan


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