Re: [PATCH v4 05/19] xen: add some dummy headers for PVH mode

[Juergen Gross]

On 08/11/2018 16:36, Daniel Kiper wrote:
> On Wed, Nov 07, 2018 at 03:49:51PM +0100, Juergen Gross wrote:
>> On 07/11/2018 13:21, Daniel Kiper wrote:
>>> On Fri, Nov 02, 2018 at 01:37:24PM +0100, Juergen Gross wrote:
>>>> With Xen PVH mode adding a new machine type the machine related headers
>>>> need to be present for the build to succeed. Most of the headers just
>>>> need to include the related common i386 headers. Add those to the tree.
>>>>
>>>> Signed-off-by: Juergen Gross <address@hidden>
>>>> ---
>>>> V3: updated commit message (Daniel Kiper)
>>>>     xenpvh->xen_pvh (Daniel Kiper)
>>>> ---
>>>>  include/grub/i386/xen_pvh/boot.h    | 1 +
>>>>  include/grub/i386/xen_pvh/console.h | 1 +
>>>>  include/grub/i386/xen_pvh/int.h     | 1 +
>>>>  include/grub/i386/xen_pvh/memory.h  | 1 +
>>>>  include/grub/i386/xen_pvh/time.h    | 1 +
>>>>  5 files changed, 5 insertions(+)
>>>>  create mode 100644 include/grub/i386/xen_pvh/boot.h
>>>>  create mode 100644 include/grub/i386/xen_pvh/console.h
>>>>  create mode 100644 include/grub/i386/xen_pvh/int.h
>>>>  create mode 100644 include/grub/i386/xen_pvh/memory.h
>>>>  create mode 100644 include/grub/i386/xen_pvh/time.h
>>>>
>>>> diff --git a/include/grub/i386/xen_pvh/boot.h 
>>>> b/include/grub/i386/xen_pvh/boot.h
>>>> new file mode 100644
>>>> index 000000000..6cd23aa83
>>>> --- /dev/null
>>>> +++ b/include/grub/i386/xen_pvh/boot.h
>>>> @@ -0,0 +1 @@
>>>> +#include <grub/i386/pc/boot.h>
>>>> diff --git a/include/grub/i386/xen_pvh/console.h 
>>>> b/include/grub/i386/xen_pvh/console.h
>>>> new file mode 100644
>>>> index 000000000..305a46d8e
>>>> --- /dev/null
>>>> +++ b/include/grub/i386/xen_pvh/console.h
>>>> @@ -0,0 +1 @@
>>>> +#include <grub/i386/pc/console.h>
>>>> diff --git a/include/grub/i386/xen_pvh/int.h 
>>>> b/include/grub/i386/xen_pvh/int.h
>>>> new file mode 100644
>>>> index 000000000..0f1f9ee62
>>>> --- /dev/null
>>>> +++ b/include/grub/i386/xen_pvh/int.h
>>>> @@ -0,0 +1 @@
>>>> +#include <grub/i386/pc/int_types.h>
>>>
>>> I think that this begs for explanation in the commit message
>>> why not grub/i386/pc/int.h.
>>
>> Okay.
>>
>>>
>>>> diff --git a/include/grub/i386/xen_pvh/memory.h 
>>>> b/include/grub/i386/xen_pvh/memory.h
>>>> new file mode 100644
>>>> index 000000000..8dd6f7c8c
>>>> --- /dev/null
>>>> +++ b/include/grub/i386/xen_pvh/memory.h
>>>> @@ -0,0 +1 @@
>>>> +#include <grub/i386/coreboot/memory.h>
>>>
>>> Hmmm... Why not include/grub/i386/pc/memory.h?
>>
>> The coreboot variant is containing the stubs grub_machine_mmap_register
>> and grub_machine_mmap_unregister I need, with the pc variant I'd have to
>> add those to xen-pvh code.
>>
>> Using the coreboot variant for that purpose seems to be common practice:
>> include/grub/i386/qemu/memory.h
>> include/grub/i386/ieee1275/memory.h
>> include/grub/i386/multiboot/memory.h
>> are doing exactly the same.
> 
> I am OK with it then. However, please say about that in the commit
> message. Otherwise it looks like a mistake.

Will do.


Juergen