Re: [Xen-devel] [PATCH v2 04/18] xen: prepare common code for Xen PVH support

[Juergen Gross]

On 18/10/2018 17:39, Juergen Gross wrote:
> On 18/10/2018 16:59, Daniel Kiper wrote:
>> On Tue, Oct 09, 2018 at 01:03:03PM +0200, Juergen Gross wrote:
>>> Some common code needs to be special cased for Xen PVH mode. This hits
>>> mostly Xen PV mode specific areas.
>>>
>>> Signed-off-by: Juergen Gross <address@hidden>
>>> ---
>>>  grub-core/kern/i386/tsc.c         | 2 +-
>>>  include/grub/i386/pc/int.h        | 3 +++
>>>  include/grub/i386/tsc.h           | 2 +-
>>>  include/grub/i386/xen/hypercall.h | 5 ++++-
>>>  include/grub/kernel.h             | 4 +++-
>>>  5 files changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c
>>> index f266eb131..43fee3a13 100644
>>> --- a/grub-core/kern/i386/tsc.c
>>> +++ b/grub-core/kern/i386/tsc.c
>>> @@ -65,7 +65,7 @@ grub_tsc_init (void)
>>>
>>>    tsc_boot_time = grub_get_tsc ();
>>>
>>> -#ifdef GRUB_MACHINE_XEN
>>> +#if defined (GRUB_MACHINE_XEN) || defined (GRUB_MACHINE_XENPVH)
>>
>> s/GRUB_MACHINE_XENPVH/GRUB_MACHINE_XEN_PVH/
> 
> Okay.
> 
>>
>>>    (void) (grub_tsc_calibrate_from_xen () || calibrate_tsc_hardcode());
>>>  #elif defined (GRUB_MACHINE_EFI)
>>>    (void) (grub_tsc_calibrate_from_pmtimer () || 
>>> grub_tsc_calibrate_from_pit () || grub_tsc_calibrate_from_efi() || 
>>> calibrate_tsc_hardcode());
>>> diff --git a/include/grub/i386/pc/int.h b/include/grub/i386/pc/int.h
>>> index 16a53e4fe..46fb1e397 100644
>>> --- a/include/grub/i386/pc/int.h
>>> +++ b/include/grub/i386/pc/int.h
>>> @@ -51,9 +51,12 @@ struct grub_bios_int_registers
>>>  #define  GRUB_CPU_INT_FLAGS_DEFAULT   0
>>>  #endif
>>>
>>> +#ifndef GRUB_MACHINE_XENPVH
>>>  void EXPORT_FUNC (grub_bios_interrupt) (grub_uint8_t intno,
>>>                                     struct grub_bios_int_registers *regs)
>>>       __attribute__ ((regparm(3)));
>>> +#endif
>>
>> Is it an issue with this declaration? I think that you should take care
>> about grub-core/kern/i386/int.S. So, relevant Makefile should be
>> updated instead of this declaration.
> 
> I'll have a try. I just remember I struggled a lot with this issue
> when writing the patches.

Ah, now I've found the problem:

I'm including grub/i386/pc/int.h from grub/i386/xenpvh/int.h in order
to avoid having to redefine all the macros and structs in that file.

Unfortunately the EXPORT_FUNC() will make the build fail as in Xen-PVH
I don't have the symbol grub_bios_interrupt available.

I see the following solutions:

1. keep the patch as it is now
2. duplicate grub/i386/pc/int.h in grub/i386/xenpvh/int.h without
   EXPORT_FUNC (grub_bios_interrupt)
3. add a dummy grub_bios_interrupt entry in Xen-PVH code
4. split grub/i386/pc/int.h into int_types.h and int.h with
   int_types.h containing the stuff I need for Xen-PVH and include
   int_types.h from grub/i386/pc/int.h and grub/i386/xenpvh/int.h
   (grub/i386/pc/int.h would contain the EXPORT_FUNC then)

What is your preference?


Juergen