| Message ID | 20191004103844.32590-1-david@gibson.dropbear.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc: Fix up RTAS invocation for new qemu versions | expand |
On 04/10/19 12:38, David Gibson wrote: > In order to call RTAS functions on powerpc kvm-unit-tests relies on the > RTAS blob supplied by qemu. But new versions of qemu don't supply an RTAS > blob: since the normal way for guests to get RTAS is to call the guest > firmware's instantiate-rtas function, we now rely on that guest firmware > to provide the RTAS code itself. > > But qemu-kvm-tests bypasses the usual guest firmware to just run itself, > so we can't get the rtas blob from SLOF. > > But.. in fact the RTAS blob under qemu is a bit of a sham anyway - it's > a tiny wrapper that forwards the RTAS call to a hypercall. So, we can > just invoke that hypercall directly. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Would it be hard to call instantiate-rtas? I'm not sure if you have any interest in running kvm-unit-tests on hypervisors that might have a different way to process RTAS calls. Paolo > --- > lib/powerpc/asm/hcall.h | 3 +++ > lib/powerpc/rtas.c | 6 +++--- > powerpc/cstart64.S | 20 ++++++++++++++++---- > 3 files changed, 22 insertions(+), 7 deletions(-) > > So.. "new versions of qemu" in this case means ones that incorporate > the pull request I just sent today. > > diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h > index a8bd7e3..1173fea 100644 > --- a/lib/powerpc/asm/hcall.h > +++ b/lib/powerpc/asm/hcall.h > @@ -24,6 +24,9 @@ > #define H_RANDOM 0x300 > #define H_SET_MODE 0x31C > > +#define KVMPPC_HCALL_BASE 0xf000 > +#define KVMPPC_H_RTAS (KVMPPC_HCALL_BASE + 0x0) > + > #ifndef __ASSEMBLY__ > /* > * hcall_have_broken_sc1 checks if we're on a host with a broken sc1. > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c > index 2e7e0da..41c0a24 100644 > --- a/lib/powerpc/rtas.c > +++ b/lib/powerpc/rtas.c > @@ -46,9 +46,9 @@ void rtas_init(void) > prop = fdt_get_property(dt_fdt(), node, > "linux,rtas-entry", &len); > if (!prop) { > - printf("%s: /rtas/linux,rtas-entry: %s\n", > - __func__, fdt_strerror(len)); > - abort(); > + /* We don't have a qemu provided RTAS blob, enter_rtas > + * will use H_RTAS directly */ > + return; > } > data = (u32 *)prop->data; > rtas_entry = (unsigned long)fdt32_to_cpu(*data); > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > index ec673b3..972851f 100644 > --- a/powerpc/cstart64.S > +++ b/powerpc/cstart64.S > @@ -121,13 +121,25 @@ halt: > > .globl enter_rtas > enter_rtas: > + LOAD_REG_ADDR(r11, rtas_entry) > + ld r10, 0(r11) > + > + cmpdi r10,0 > + bne external_rtas > + > + /* Use H_RTAS directly */ > + mr r4,r3 > + lis r3,KVMPPC_H_RTAS@h > + ori r3,r3,KVMPPC_H_RTAS@l > + b hcall > + > +external_rtas: > + /* Use external RTAS blob */ > mflr r0 > std r0, 16(r1) > > - LOAD_REG_ADDR(r10, rtas_return_loc) > - mtlr r10 > - LOAD_REG_ADDR(r11, rtas_entry) > - ld r10, 0(r11) > + LOAD_REG_ADDR(r11, rtas_return_loc) > + mtlr r11 > > mfmsr r11 > LOAD_REG_IMMEDIATE(r9, RTAS_MSR_MASK) >
On Fri, Oct 04, 2019 at 01:54:35PM +0200, Paolo Bonzini wrote: > On 04/10/19 12:38, David Gibson wrote: > > In order to call RTAS functions on powerpc kvm-unit-tests relies on the > > RTAS blob supplied by qemu. But new versions of qemu don't supply an RTAS > > blob: since the normal way for guests to get RTAS is to call the guest > > firmware's instantiate-rtas function, we now rely on that guest firmware > > to provide the RTAS code itself. > > > > But qemu-kvm-tests bypasses the usual guest firmware to just run itself, > > so we can't get the rtas blob from SLOF. > > > > But.. in fact the RTAS blob under qemu is a bit of a sham anyway - it's > > a tiny wrapper that forwards the RTAS call to a hypercall. So, we can > > just invoke that hypercall directly. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Would it be hard to call instantiate-rtas? Fairly. First, we'd have to not use -bios to replace the normal SLOF firmware - that could significantly slow down tests, since SLOF itself takes a little while to start up. Then we'd need to get the unit tests to boot out of slof which is a different interface from the raw one we're using now. Then we'd need to get it to use the OF client interface to call instantiate-rtas - we currently don't ever call OF (obviously, since we don't even load it). > I'm not sure if you have any > interest in running kvm-unit-tests on hypervisors that might have a > different way to process RTAS calls. Probably not..? > > Paolo > > > --- > > lib/powerpc/asm/hcall.h | 3 +++ > > lib/powerpc/rtas.c | 6 +++--- > > powerpc/cstart64.S | 20 ++++++++++++++++---- > > 3 files changed, 22 insertions(+), 7 deletions(-) > > > > So.. "new versions of qemu" in this case means ones that incorporate > > the pull request I just sent today. > > > > diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h > > index a8bd7e3..1173fea 100644 > > --- a/lib/powerpc/asm/hcall.h > > +++ b/lib/powerpc/asm/hcall.h > > @@ -24,6 +24,9 @@ > > #define H_RANDOM 0x300 > > #define H_SET_MODE 0x31C > > > > +#define KVMPPC_HCALL_BASE 0xf000 > > +#define KVMPPC_H_RTAS (KVMPPC_HCALL_BASE + 0x0) > > + > > #ifndef __ASSEMBLY__ > > /* > > * hcall_have_broken_sc1 checks if we're on a host with a broken sc1. > > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c > > index 2e7e0da..41c0a24 100644 > > --- a/lib/powerpc/rtas.c > > +++ b/lib/powerpc/rtas.c > > @@ -46,9 +46,9 @@ void rtas_init(void) > > prop = fdt_get_property(dt_fdt(), node, > > "linux,rtas-entry", &len); > > if (!prop) { > > - printf("%s: /rtas/linux,rtas-entry: %s\n", > > - __func__, fdt_strerror(len)); > > - abort(); > > + /* We don't have a qemu provided RTAS blob, enter_rtas > > + * will use H_RTAS directly */ > > + return; > > } > > data = (u32 *)prop->data; > > rtas_entry = (unsigned long)fdt32_to_cpu(*data); > > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > > index ec673b3..972851f 100644 > > --- a/powerpc/cstart64.S > > +++ b/powerpc/cstart64.S > > @@ -121,13 +121,25 @@ halt: > > > > .globl enter_rtas > > enter_rtas: > > + LOAD_REG_ADDR(r11, rtas_entry) > > + ld r10, 0(r11) > > + > > + cmpdi r10,0 > > + bne external_rtas > > + > > + /* Use H_RTAS directly */ > > + mr r4,r3 > > + lis r3,KVMPPC_H_RTAS@h > > + ori r3,r3,KVMPPC_H_RTAS@l > > + b hcall > > + > > +external_rtas: > > + /* Use external RTAS blob */ > > mflr r0 > > std r0, 16(r1) > > > > - LOAD_REG_ADDR(r10, rtas_return_loc) > > - mtlr r10 > > - LOAD_REG_ADDR(r11, rtas_entry) > > - ld r10, 0(r11) > > + LOAD_REG_ADDR(r11, rtas_return_loc) > > + mtlr r11 > > > > mfmsr r11 > > LOAD_REG_IMMEDIATE(r9, RTAS_MSR_MASK) > > >
On 05/10/19 10:11, David Gibson wrote: > Fairly. First, we'd have to not use -bios to replace the normal SLOF > firmware - that could significantly slow down tests, since SLOF itself > takes a little while to start up. Oh, I forgot that PPC kvm-unit-tests use -bios. That makes everything moot. Paolo
On Sun, Oct 06, 2019 at 06:16:17PM +0200, Paolo Bonzini wrote: > On 05/10/19 10:11, David Gibson wrote: > > Fairly. First, we'd have to not use -bios to replace the normal SLOF > > firmware - that could significantly slow down tests, since SLOF itself > > takes a little while to start up. > > Oh, I forgot that PPC kvm-unit-tests use -bios. That makes > everything moot. Right. I did consider putting a closer-to-conventional RTAS blob in kbm-unit-tests' special "firmware" (which currently consists of a single jump instruction). It does help to build it BE which RTAS expects. On the other hand, I wasn't about to implement a full OF client interface, so it would still have the main part of the tests relying on finding the blob at some well known location. And finding a well known location that we can count on the tests proper not clobbering is a bit of a pain as well.
On 04/10/2019 12.38, David Gibson wrote: > In order to call RTAS functions on powerpc kvm-unit-tests relies on the > RTAS blob supplied by qemu. But new versions of qemu don't supply an RTAS > blob: since the normal way for guests to get RTAS is to call the guest > firmware's instantiate-rtas function, we now rely on that guest firmware > to provide the RTAS code itself. > > But qemu-kvm-tests bypasses the usual guest firmware to just run itself, s/qemu-kvm-tests/kvm-unit-tests/ > so we can't get the rtas blob from SLOF. > > But.. in fact the RTAS blob under qemu is a bit of a sham anyway - it's > a tiny wrapper that forwards the RTAS call to a hypercall. So, we can > just invoke that hypercall directly. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > lib/powerpc/asm/hcall.h | 3 +++ > lib/powerpc/rtas.c | 6 +++--- > powerpc/cstart64.S | 20 ++++++++++++++++---- > 3 files changed, 22 insertions(+), 7 deletions(-) > > So.. "new versions of qemu" in this case means ones that incorporate > the pull request I just sent today. > > diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h > index a8bd7e3..1173fea 100644 > --- a/lib/powerpc/asm/hcall.h > +++ b/lib/powerpc/asm/hcall.h > @@ -24,6 +24,9 @@ > #define H_RANDOM 0x300 > #define H_SET_MODE 0x31C > > +#define KVMPPC_HCALL_BASE 0xf000 > +#define KVMPPC_H_RTAS (KVMPPC_HCALL_BASE + 0x0) > + > #ifndef __ASSEMBLY__ > /* > * hcall_have_broken_sc1 checks if we're on a host with a broken sc1. > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c > index 2e7e0da..41c0a24 100644 > --- a/lib/powerpc/rtas.c > +++ b/lib/powerpc/rtas.c > @@ -46,9 +46,9 @@ void rtas_init(void) > prop = fdt_get_property(dt_fdt(), node, > "linux,rtas-entry", &len); > if (!prop) { > - printf("%s: /rtas/linux,rtas-entry: %s\n", > - __func__, fdt_strerror(len)); > - abort(); > + /* We don't have a qemu provided RTAS blob, enter_rtas > + * will use H_RTAS directly */ > + return; > } > data = (u32 *)prop->data; > rtas_entry = (unsigned long)fdt32_to_cpu(*data); > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > index ec673b3..972851f 100644 > --- a/powerpc/cstart64.S > +++ b/powerpc/cstart64.S > @@ -121,13 +121,25 @@ halt: > > .globl enter_rtas > enter_rtas: > + LOAD_REG_ADDR(r11, rtas_entry) > + ld r10, 0(r11) > + > + cmpdi r10,0 > + bne external_rtas > + > + /* Use H_RTAS directly */ > + mr r4,r3 > + lis r3,KVMPPC_H_RTAS@h > + ori r3,r3,KVMPPC_H_RTAS@l > + b hcall > + > +external_rtas: > + /* Use external RTAS blob */ > mflr r0 > std r0, 16(r1) > > - LOAD_REG_ADDR(r10, rtas_return_loc) > - mtlr r10 > - LOAD_REG_ADDR(r11, rtas_entry) > - ld r10, 0(r11) > + LOAD_REG_ADDR(r11, rtas_return_loc) > + mtlr r11 > > mfmsr r11 > LOAD_REG_IMMEDIATE(r9, RTAS_MSR_MASK) > With the typo in the commit message fixed: Reviewed-by: Thomas Huth <thuth@redhat.com> Paolo, I currently don't have any other kvm-unit-test patches pending, could you pick this up directly, please?
On 09/10/19 12:35, Thomas Huth wrote: > On 04/10/2019 12.38, David Gibson wrote: >> In order to call RTAS functions on powerpc kvm-unit-tests relies on the >> RTAS blob supplied by qemu. But new versions of qemu don't supply an RTAS >> blob: since the normal way for guests to get RTAS is to call the guest >> firmware's instantiate-rtas function, we now rely on that guest firmware >> to provide the RTAS code itself. >> >> But qemu-kvm-tests bypasses the usual guest firmware to just run itself, > > s/qemu-kvm-tests/kvm-unit-tests/ > >> so we can't get the rtas blob from SLOF. >> >> But.. in fact the RTAS blob under qemu is a bit of a sham anyway - it's >> a tiny wrapper that forwards the RTAS call to a hypercall. So, we can >> just invoke that hypercall directly. >> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >> --- >> lib/powerpc/asm/hcall.h | 3 +++ >> lib/powerpc/rtas.c | 6 +++--- >> powerpc/cstart64.S | 20 ++++++++++++++++---- >> 3 files changed, 22 insertions(+), 7 deletions(-) >> >> So.. "new versions of qemu" in this case means ones that incorporate >> the pull request I just sent today. >> >> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h >> index a8bd7e3..1173fea 100644 >> --- a/lib/powerpc/asm/hcall.h >> +++ b/lib/powerpc/asm/hcall.h >> @@ -24,6 +24,9 @@ >> #define H_RANDOM 0x300 >> #define H_SET_MODE 0x31C >> >> +#define KVMPPC_HCALL_BASE 0xf000 >> +#define KVMPPC_H_RTAS (KVMPPC_HCALL_BASE + 0x0) >> + >> #ifndef __ASSEMBLY__ >> /* >> * hcall_have_broken_sc1 checks if we're on a host with a broken sc1. >> diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c >> index 2e7e0da..41c0a24 100644 >> --- a/lib/powerpc/rtas.c >> +++ b/lib/powerpc/rtas.c >> @@ -46,9 +46,9 @@ void rtas_init(void) >> prop = fdt_get_property(dt_fdt(), node, >> "linux,rtas-entry", &len); >> if (!prop) { >> - printf("%s: /rtas/linux,rtas-entry: %s\n", >> - __func__, fdt_strerror(len)); >> - abort(); >> + /* We don't have a qemu provided RTAS blob, enter_rtas >> + * will use H_RTAS directly */ >> + return; >> } >> data = (u32 *)prop->data; >> rtas_entry = (unsigned long)fdt32_to_cpu(*data); >> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S >> index ec673b3..972851f 100644 >> --- a/powerpc/cstart64.S >> +++ b/powerpc/cstart64.S >> @@ -121,13 +121,25 @@ halt: >> >> .globl enter_rtas >> enter_rtas: >> + LOAD_REG_ADDR(r11, rtas_entry) >> + ld r10, 0(r11) >> + >> + cmpdi r10,0 >> + bne external_rtas >> + >> + /* Use H_RTAS directly */ >> + mr r4,r3 >> + lis r3,KVMPPC_H_RTAS@h >> + ori r3,r3,KVMPPC_H_RTAS@l >> + b hcall >> + >> +external_rtas: >> + /* Use external RTAS blob */ >> mflr r0 >> std r0, 16(r1) >> >> - LOAD_REG_ADDR(r10, rtas_return_loc) >> - mtlr r10 >> - LOAD_REG_ADDR(r11, rtas_entry) >> - ld r10, 0(r11) >> + LOAD_REG_ADDR(r11, rtas_return_loc) >> + mtlr r11 >> >> mfmsr r11 >> LOAD_REG_IMMEDIATE(r9, RTAS_MSR_MASK) >> > > With the typo in the commit message fixed: > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Paolo, I currently don't have any other kvm-unit-test patches pending, > could you pick this up directly, please? Ok, thanks. Paolo
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h index a8bd7e3..1173fea 100644 --- a/lib/powerpc/asm/hcall.h +++ b/lib/powerpc/asm/hcall.h @@ -24,6 +24,9 @@ #define H_RANDOM 0x300 #define H_SET_MODE 0x31C +#define KVMPPC_HCALL_BASE 0xf000 +#define KVMPPC_H_RTAS (KVMPPC_HCALL_BASE + 0x0) + #ifndef __ASSEMBLY__ /* * hcall_have_broken_sc1 checks if we're on a host with a broken sc1. diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c index 2e7e0da..41c0a24 100644 --- a/lib/powerpc/rtas.c +++ b/lib/powerpc/rtas.c @@ -46,9 +46,9 @@ void rtas_init(void) prop = fdt_get_property(dt_fdt(), node, "linux,rtas-entry", &len); if (!prop) { - printf("%s: /rtas/linux,rtas-entry: %s\n", - __func__, fdt_strerror(len)); - abort(); + /* We don't have a qemu provided RTAS blob, enter_rtas + * will use H_RTAS directly */ + return; } data = (u32 *)prop->data; rtas_entry = (unsigned long)fdt32_to_cpu(*data); diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S index ec673b3..972851f 100644 --- a/powerpc/cstart64.S +++ b/powerpc/cstart64.S @@ -121,13 +121,25 @@ halt: .globl enter_rtas enter_rtas: + LOAD_REG_ADDR(r11, rtas_entry) + ld r10, 0(r11) + + cmpdi r10,0 + bne external_rtas + + /* Use H_RTAS directly */ + mr r4,r3 + lis r3,KVMPPC_H_RTAS@h + ori r3,r3,KVMPPC_H_RTAS@l + b hcall + +external_rtas: + /* Use external RTAS blob */ mflr r0 std r0, 16(r1) - LOAD_REG_ADDR(r10, rtas_return_loc) - mtlr r10 - LOAD_REG_ADDR(r11, rtas_entry) - ld r10, 0(r11) + LOAD_REG_ADDR(r11, rtas_return_loc) + mtlr r11 mfmsr r11 LOAD_REG_IMMEDIATE(r9, RTAS_MSR_MASK)
In order to call RTAS functions on powerpc kvm-unit-tests relies on the RTAS blob supplied by qemu. But new versions of qemu don't supply an RTAS blob: since the normal way for guests to get RTAS is to call the guest firmware's instantiate-rtas function, we now rely on that guest firmware to provide the RTAS code itself. But qemu-kvm-tests bypasses the usual guest firmware to just run itself, so we can't get the rtas blob from SLOF. But.. in fact the RTAS blob under qemu is a bit of a sham anyway - it's a tiny wrapper that forwards the RTAS call to a hypercall. So, we can just invoke that hypercall directly. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- lib/powerpc/asm/hcall.h | 3 +++ lib/powerpc/rtas.c | 6 +++--- powerpc/cstart64.S | 20 ++++++++++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) So.. "new versions of qemu" in this case means ones that incorporate the pull request I just sent today.