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

[Daniel Kiper]

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.

Daniel