Re: [Xen-devel] [PATCH v4 10/19] xen: setup hypercall page for PVH
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: [PATCH v4 10/19] xen: setup hypercall page for PVH
On 09/11/2018 19:12, Daniel Kiper 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
>> ---
>> 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
>
> I think that this is not PVH specific thing. Could you move this to
> more generic Xen header?
>
>> grub_uint64_t grub_rsdp_addr;
>
> Hmmm... It seems to me that immediately after patch #8 GRUB2 build is
> broken. Is not it?
No. xen-pvh isn't yet selectable.
>
>> +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 not you do something similar to grub-core/lib/i386/halt.c:stop() here?
Yes, as already replied to Roger.
>
>> +}
>> +
>> +static void
>> +grub_xen_cpuid_base (void)
>> +{
>> + grub_uint32_t base, eax, signature[3];
>> +
>> + for (base = 0x4000; base < 0x4001; 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_panic ("Found no Xen signature.\n");
>
> "Found no Xen signature!\nSystem halted!\n"
>
> Or maybe grub_xen_panic() should always add "System halted!\n".
Okay (the latter).
>
>> +}
>> +
>> +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) (_page[0]);
>
> Could not you use hypercall_page alone here?
Hmm, yes.
>
>> +
>> + 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.
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
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
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
Re: [PATCH v4 10/19] xen: setup hypercall page for PVH
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
> ---
> 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
I think that this is not PVH specific thing. Could you move this to
more generic Xen header?
> grub_uint64_t grub_rsdp_addr;
Hmmm... It seems to me that immediately after patch #8 GRUB2 build is
broken. Is not it?
> +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 not you do something similar to grub-core/lib/i386/halt.c:stop() here?
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> + grub_uint32_t base, eax, signature[3];
> +
> + for (base = 0x4000; base < 0x4001; 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_panic ("Found no Xen signature.\n");
"Found no Xen signature!\nSystem halted!\n"
Or maybe grub_xen_panic() should always add "System halted!\n".
> +}
> +
> +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) (_page[0]);
Could not you use hypercall_page alone here?
> +
> + 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.
> + [callno] "a" (_page[callno * 32])
> + : "memory");
> + return __res;
> +}
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 10/19] xen: setup hypercall page for PVH
Add the needed code to setup the hypercall page for calling into the
Xen hypervisor.
Signed-off-by: Juergen Gross
---
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
+
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");
+}
+
+static void
+grub_xen_cpuid_base (void)
+{
+ grub_uint32_t base, eax, signature[3];
+
+ for (base = 0x4000; base < 0x4001; 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_panic ("Found no Xen signature.\n");
+}
+
+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) (_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)))
+{
+ 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),
+ [callno] "a" (_page[callno * 32])
+ : "memory");
+ return __res;
+}
+
void
grub_xen_setup_pvh (void)
{
+ grub_xen_cpuid_base ();
+ grub_xen_setup_hypercall_page ();
}
grub_err_t
--
2.16.4
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
