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

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

On 06/06/2016 07:31 AM, Jan Beulich wrote:
>>>> On 06.04.16 at 03:25, <boris.ostrovsky@oracle.com> wrote:
>> Users of ACPI builder will need to provide their own implemetations of
>> strncpy(), memcpy, memset() and printf declared in stdio.h and string.h.
>> For hvmloader we provide those two as wrappers around utul.h.
>>
>> Some x86-specific definitions move to ACPI builder's x86.h file so that
>> users won't have to re-define them themselves.
>>
>> Explicitly define a few things that are currently available to to build.c
>> via util.h.
> Considering the first paragraph, this last sentence was more
> confusing than clarifying to me: I expected util.h to get changed
> in some way, but aiui having gone through the entire patch this is
> just a (very different) re-statement of what you've said already.
>
>>  create mode 100644 tools/firmware/hvmloader/acpi/x86.h
>>  create mode 100644 tools/firmware/hvmloader/stdio.h
>>  create mode 100644 tools/firmware/hvmloader/string.h
> I have to admit that I'm not really looking forward to see the
> hypervisor gain a file stdio.h. I'm also not convinced libxc'e
> planned use of the libacpi/ would fit well with uses of printf() in
> that subtree.

Hypervisor won't use it, it will be included under '#ifndef __XEN__' (in
patch 20).

>
>> --- /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?

-boris


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