[PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[Ross Philipson]

[-- Attachment #1: Type: text/plain, Size: 158 bytes --]

Fetch module set base address from ECX:EDX and call the module
support initialization routine.

Signed-off-by: Ross Philipson <ross.philipson@citrix.com>

[-- Attachment #2: hvm-firmware-passthrough-03.patch --]
[-- Type: application/octet-stream, Size: 1248 bytes --]

# HG changeset patch
# Parent 45d2dcc22c1824d8f1b1a26ac1bf3ea3f7e270b2
Fetch module set base address from ECX:EDX and call the module
support initialization routine.

Signed-off-by: Ross Philipson <ross.philipson@citrix.com>

diff -r 45d2dcc22c18 tools/firmware/hvmloader/hvmloader.c
--- a/tools/firmware/hvmloader/hvmloader.c	Mon Mar 19 16:42:36 2012 -0400
+++ b/tools/firmware/hvmloader/hvmloader.c	Mon Mar 19 16:45:12 2012 -0400
@@ -23,6 +23,7 @@
 #include "util.h"
 #include "hypercall.h"
 #include "config.h"
+#include "modules.h"
 #include "pci_regs.h"
 #include "apic_regs.h"
 #include "acpi/acpi2_0.h"
@@ -257,6 +258,17 @@ int main(void)
 {
     const struct bios_config *bios;
     int acpi_enabled;
+    uint32_t mod_lo, mod_hi;
+    uint64_t mod_base;
+
+    /* First get the modules base address passed in ECX:EDC and init
+     * module support.
+     */
+    asm volatile ( "mov %%ecx, %0;" : "=r"(mod_hi));
+    asm volatile ( "mov %%edx, %0;" : "=r"(mod_lo));
+    mod_base = mod_hi;
+    mod_base = (0xFFFFFFFF00000000 & (mod_base << 32)) | mod_lo;
+    init_hvm_modules(mod_base);
 
     /* Initialise hypercall stubs with RET, rendering them no-ops. */
     memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[Ian Campbell]

On Mon, 2012-03-19 at 22:04 +0000, Ross Philipson wrote:
> diff -r 45d2dcc22c18 tools/firmware/hvmloader/hvmloader.c
> --- a/tools/firmware/hvmloader/hvmloader.c      Mon Mar 19 16:42:36
> 2012 -0400
> +++ b/tools/firmware/hvmloader/hvmloader.c      Mon Mar 19 16:45:12
> 2012 -0400
> @@ -23,6 +23,7 @@
>  #include "util.h"
>  #include "hypercall.h"
>  #include "config.h"
> +#include "modules.h"
>  #include "pci_regs.h"
>  #include "apic_regs.h"
>  #include "acpi/acpi2_0.h"
> @@ -257,6 +258,17 @@ int main(void)
>  {
>      const struct bios_config *bios;
>      int acpi_enabled;
> +    uint32_t mod_lo, mod_hi;
> +    uint64_t mod_base;
> +
> +    /* First get the modules base address passed in ECX:EDC and init
> +     * module support.
> +     */
> +    asm volatile ( "mov %%ecx, %0;" : "=r"(mod_hi));
> +    asm volatile ( "mov %%edx, %0;" : "=r"(mod_lo));

I'm not sure you can rely on %ecx and %edx not having been clobbered
here. I think you probably need to save away the value in the ASM block
at the head of hvmloader.c if we continue down this path.

In fact, it looks like that ASM block deliberately zeroes ecx and edx so how
does this work?

> +    mod_base = mod_hi;
> +    mod_base = (0xFFFFFFFF00000000 & (mod_base << 32)) | mod_lo;
> +    init_hvm_modules(mod_base);
>  
>      /* Initialise hypercall stubs with RET, rendering them no-ops. */
>      memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */,
> PAGE_SIZE); 

Re: [PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[Tim Deegan]

At 10:30 +0100 on 04 Apr (1333535447), Ian Campbell wrote:
> On Mon, 2012-03-19 at 22:04 +0000, Ross Philipson wrote:
> > diff -r 45d2dcc22c18 tools/firmware/hvmloader/hvmloader.c
> > --- a/tools/firmware/hvmloader/hvmloader.c      Mon Mar 19 16:42:36
> > 2012 -0400
> > +++ b/tools/firmware/hvmloader/hvmloader.c      Mon Mar 19 16:45:12
> > 2012 -0400
> > @@ -23,6 +23,7 @@
> >  #include "util.h"
> >  #include "hypercall.h"
> >  #include "config.h"
> > +#include "modules.h"
> >  #include "pci_regs.h"
> >  #include "apic_regs.h"
> >  #include "acpi/acpi2_0.h"
> > @@ -257,6 +258,17 @@ int main(void)
> >  {
> >      const struct bios_config *bios;
> >      int acpi_enabled;
> > +    uint32_t mod_lo, mod_hi;
> > +    uint64_t mod_base;
> > +
> > +    /* First get the modules base address passed in ECX:EDC and init
> > +     * module support.
> > +     */
> > +    asm volatile ( "mov %%ecx, %0;" : "=r"(mod_hi));
> > +    asm volatile ( "mov %%edx, %0;" : "=r"(mod_lo));
> 
> I'm not sure you can rely on %ecx and %edx not having been clobbered
> here. I think you probably need to save away the value in the ASM block
> at the head of hvmloader.c if we continue down this path.
> 
> In fact, it looks like that ASM block deliberately zeroes ecx and edx so how
> does this work?

The asm header clears them after calling main().  But yes, you can't
rely on their being still valid; the asm header would have to push them
to the stack as arguments to main().

In any case, this all goes away if the module info is passed in xenstore.

Cheers,

Tim.

Re: [PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[Ian Campbell]

On Wed, 2012-04-04 at 10:47 +0100, Tim Deegan wrote:
[...]
> > In fact, it looks like that ASM block deliberately zeroes ecx and edx so how
> > does this work?
> 
> The asm header clears them after calling main().

So it does, I didn't realise that the "go" button in hvmloader was
return from main, subtle!

[...]
> In any case, this all goes away if the module info is passed in xenstore.

Yes.

Ian.

Re: [PATCH 03/07] HVM firmware passthrough: hvmloader init module support

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Wednesday, April 04, 2012 5:51 AM
> To: Tim (Xen.org)
> Cc: Ross Philipson; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 03/07] HVM firmware passthrough:
> hvmloader init module support
> 
> On Wed, 2012-04-04 at 10:47 +0100, Tim Deegan wrote:
> [...]
> > > In fact, it looks like that ASM block deliberately zeroes ecx and
> > > edx so how does this work?
> >
> > The asm header clears them after calling main().
> 
> So it does, I didn't realise that the "go" button in hvmloader was
> return from main, subtle!
> 
> [...]
> > In any case, this all goes away if the module info is passed in
> xenstore.
> 
> Yes.
> 
> Ian.
> 

I mainly did it this way as a first approach since I was not sure how to use xenstore with libxc etc. I figured this would be called out for critique anyway. But, as noted, it is no longer an issue.