Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH

[Juergen Gross]

On 19/10/2018 14:30, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
>> Add the needed code to setup the hypercall page for calling into the
>> Xen hypervisor.
>>
>> Signed-off-by: Juergen Gross <address@hidden>
>> ---
>>  grub-core/kern/i386/xen/pvh.c | 70 
>> +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index 182ef95f9..c1b1cf8db 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -20,14 +20,84 @@
>>  #include <grub/misc.h>
>>  #include <grub/memory.h>
>>  #include <grub/mm.h>
>> +#include <grub/i386/cpuid.h>
>>  #include <grub/xen.h>
>>  #include <grub/machine/kernel.h>
>>
>>  grub_uint64_t grub_rsdp_addr;
>>
>> +static struct { char _entry[32]; } hypercall_page[128]
>> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>> +
>> +static grub_uint32_t xen_cpuid_base;
>> +
>> +static void
>> +grub_xen_early_halt (void)
>> +{
>> +  asm volatile ("hlt");
>> +}
>> +
>> +static void
>> +grub_xen_cpuid_base (void)
>> +{
>> +  grub_uint32_t base, eax, signature[3];
>> +
>> +  for (base = 0x40000000; base < 0x40010000; base += 0x100)
>> +    {
>> +      grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
>> +      if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
>> +    {
>> +      xen_cpuid_base = base;
>> +      return;
>> +    }
>> +    }
>> +
>> +  grub_xen_early_halt ();
>> +}
>> +
>> +static void
>> +grub_xen_setup_hypercall_page (void)
>> +{
>> +  grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
>> +
>> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
>> +  msr = ebx;
>> +  pfn = (grub_uint32_t) (&hypercall_page[0]);
>> +
>> +  asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
>> +}
>> +
>> +int
>> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
>> +                grub_uint32_t a1, grub_uint32_t a2,
>> +                grub_uint32_t a3, grub_uint32_t a4,
>> +                grub_uint32_t a5 __attribute__ ((unused)))
>> +{
>> +  register unsigned long __res  asm("eax");
>> +  register unsigned long __arg0 asm("ebx") = __arg0;
>> +  register unsigned long __arg1 asm("ecx") = __arg1;
>> +  register unsigned long __arg2 asm("edx") = __arg2;
>> +  register unsigned long __arg3 asm("esi") = __arg3;
>> +  register unsigned long __arg4 asm("edi") = __arg4;
> 
> Why do we need this play with registers. Does not asm below
> work with "a", "b", ... like above?

Probably, yes. This was inspired by Linux 32-bit version.

I can use the mini-os variant which combines Andrew's suggestion
and yours.


Juergen