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

2018-11-21 Thread Juergen Gross
On 20/11/2018 10:27, Juergen Gross wrote:
> On 09/11/2018 19:12, Daniel Kiper wrote:
>> On Fri, Nov 02, 2018 at 01:37:29PM +0100, Juergen Gross wrote:>>> +
>>> +  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)))
>>> +{
>>> +  grub_uint32_t __res, __ign0, __ign1, __ign2, __ign3, __ign4;
>>> +
>>> +  asm volatile ("call *%[callno]"
>>> +   : "=a" (__res), "=b" (__ign0), "=c" (__ign1), "=d" (__ign2),
>>> + "=S" (__ign3), "=D" (__ign4)
>>> +   : "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),
>>
>> I think that you can drop all __ign* variables if you specify proper
>> registers in input argument. If this does not work you can use "+"
>> modifier instead of "=" in the output argument.
> 
> I'll have a try.

Doesn't work.

Specifying only the input registers neglects the hypercall
clobbering those.

Using the "+" modifier lets gcc complain.

I'll just use a single dummy variable.


Juergen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2018-11-20 Thread Juergen Gross
On 15/11/2018 09:33, Roger Pau Monné wrote:
> On Fri, Nov 02, 2018 at 01:37:29PM +0100, Juergen Gross wrote:
>> Add the needed code to setup the hypercall page for calling into the
>> Xen hypervisor.
>>
>> Signed-off-by: Juergen Gross 
> 
> Thanks, LGTM, just one suggestion and one question.
> 
>> ---
>> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
>> issue panic message (Roger Pau Monné)
>> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
>> ---
>>  grub-core/kern/i386/xen/pvh.c | 69 
>> +++
>>  1 file changed, 69 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index ac6181f4e..7e90a4538 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -20,14 +20,83 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>>  
>> +#define XEN_CONSOLE_PORT   0xe9
> 
> This is now part of the public headers (XEN_HVM_DEBUGCONS_IOPORT) if
> you wish to pick it up from there.

I don't want to import headers from Xen unstable, so I'll add just that
define (and the associated comment) to include/xen/arch-x86/xen.h

> 
>> +
>>  grub_uint64_t grub_rsdp_addr;
>>  
>> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
>> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>> +
>> +static grub_uint32_t xen_cpuid_base;
>> +
>> +static void
>> +grub_xen_panic (const char *msg)
>> +{
>> +  const char *c;
>> +
>> +  for (c = msg; *c; c++)
>> +grub_outb (*c, XEN_CONSOLE_PORT);
>> +
>> +  asm volatile ("hlt");
> 
> Should this be "cli; hlt;", or does grub always runs with interrupt flag
> clear?

Good point. Maybe I should even add a loop around the hlt.


Juergen


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2018-11-15 Thread Roger Pau Monné
On Fri, Nov 02, 2018 at 01:37:29PM +0100, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
> 
> Signed-off-by: Juergen Gross 

Thanks, LGTM, just one suggestion and one question.

> ---
> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
> issue panic message (Roger Pau Monné)
> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 69 
> +++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index ac6181f4e..7e90a4538 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,14 +20,83 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  
> +#define XEN_CONSOLE_PORT   0xe9

This is now part of the public headers (XEN_HVM_DEBUGCONS_IOPORT) if
you wish to pick it up from there.

> +
>  grub_uint64_t grub_rsdp_addr;
>  
> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_panic (const char *msg)
> +{
> +  const char *c;
> +
> +  for (c = msg; *c; c++)
> +grub_outb (*c, XEN_CONSOLE_PORT);
> +
> +  asm volatile ("hlt");

Should this be "cli; hlt;", or does grub always runs with interrupt flag
clear?

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel