| Message ID | 20131212022822.GI2871@cbox |
|---|---|
| State | New |
| Headers | show |
On 12 December 2013 02:28, Christoffer Dall <christoffer.dall@linaro.org> wrote: > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > index a30035d..9565e6a 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -1889,9 +1889,12 @@ value in the kvm_regs structure seen as a 32bit array. > arm64 CCSIDR registers are demultiplexed by CSSELR value: > 0x6020 0000 0011 00 <csselr:8> > > -arm64 system registers have the following id bit patterns: > +arm64 64-bit system registers have the following id bit patterns: > 0x6030 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> > > +arm64 32-bit system registers have the following id bit patterns: > + 0x6020 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> What does it mean to say that a system register for AArch64 is "32 bits" given that MRS/MSR always operate on a 64 bit register? We have the distinction in AArch32 because the instructions (and whether the input/output is in one register or a register pair) are different, but I can't see the need for AArch64. (The code I've just written for QEMU to handle sysregs says "they're all 64 bit"...) thanks -- PMM
On 12/12/2013 10:23 AM, Peter Maydell wrote: > On 12 December 2013 02:28, Christoffer Dall <christoffer.dall@linaro.org> wrote: >> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt >> index a30035d..9565e6a 100644 >> --- a/Documentation/virtual/kvm/api.txt >> +++ b/Documentation/virtual/kvm/api.txt >> @@ -1889,9 +1889,12 @@ value in the kvm_regs structure seen as a 32bit array. >> arm64 CCSIDR registers are demultiplexed by CSSELR value: >> 0x6020 0000 0011 00 <csselr:8> >> >> -arm64 system registers have the following id bit patterns: >> +arm64 64-bit system registers have the following id bit patterns: >> 0x6030 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> >> >> +arm64 32-bit system registers have the following id bit patterns: >> + 0x6020 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> > > What does it mean to say that a system register for AArch64 > is "32 bits" given that MRS/MSR always operate on a 64 bit > register? We have the distinction in AArch32 because the > instructions (and whether the input/output is in one register > or a register pair) are different, but I can't see the need for > AArch64. But ARMv8 ARM still defines these registers as 32-bit: D8.5.14: CNTV_CTL_EL0 Attributes CNTV_CTL_EL0 is a 32-bit register. But indeed the MSR/MRS instruction references a Xt register, and the documentation does not seem to tell how this is handled, so I assume this is zero-extended. Would be great to have this clarified, though. Regards, Andre. > > (The code I've just written for QEMU to handle sysregs says > "they're all 64 bit"...) > > thanks > -- PMM >
On 12 December 2013 09:32, Andre Przywara <andre.przywara@linaro.org> wrote: > On 12/12/2013 10:23 AM, Peter Maydell wrote: >> What does it mean to say that a system register for AArch64 >> is "32 bits" given that MRS/MSR always operate on a 64 bit >> register? > But ARMv8 ARM still defines these registers as 32-bit: > D8.5.14: CNTV_CTL_EL0 > Attributes > CNTV_CTL_EL0 is a 32-bit register. > But indeed the MSR/MRS instruction references a Xt register, and the > documentation does not seem to tell how this is handled, so I assume this is > zero-extended. I checked, and for AArch64 registers, "32 bits" is just a shorthand for "64 bit register where the top 32 bits are RAZ/WI" (and I suspect it's not totally impossible that some future architecture revision might define new bits in the top half). So I would suggest that we should make the KVM user<->kernel interface just consistently make all the sysregs 64 bit. (There is actually precedent of a sort here in that the kernel claims the PSTATE register is 64 bits wide despite it really being a 32 bit SPSR format value under the hood.) thanks -- PMM
On 12/12/13 11:36, Peter Maydell wrote: > On 12 December 2013 09:32, Andre Przywara <andre.przywara@linaro.org> wrote: >> On 12/12/2013 10:23 AM, Peter Maydell wrote: >>> What does it mean to say that a system register for AArch64 >>> is "32 bits" given that MRS/MSR always operate on a 64 bit >>> register? > >> But ARMv8 ARM still defines these registers as 32-bit: >> D8.5.14: CNTV_CTL_EL0 >> Attributes >> CNTV_CTL_EL0 is a 32-bit register. >> But indeed the MSR/MRS instruction references a Xt register, and the >> documentation does not seem to tell how this is handled, so I assume this is >> zero-extended. > > I checked, and for AArch64 registers, "32 bits" is just > a shorthand for "64 bit register where the top 32 bits are > RAZ/WI" (and I suspect it's not totally impossible that some > future architecture revision might define new bits in the > top half). Indeed. Actually, there isn't an instruction to access these 32bit registers with a 'W' register. You really have to use a 'X'. > So I would suggest that we should make the KVM user<->kernel > interface just consistently make all the sysregs 64 bit. > > (There is actually precedent of a sort here in that the > kernel claims the PSTATE register is 64 bits wide despite > it really being a 32 bit SPSR format value under the hood.) I definitely agree with Peter here. I'd like to keep the ABI 64bit for the sysregs. It makes the whole thing much nicer. Thanks, M.
On Thu, Dec 12, 2013 at 05:15:36PM +0000, Marc Zyngier wrote: > On 12/12/13 11:36, Peter Maydell wrote: > > On 12 December 2013 09:32, Andre Przywara <andre.przywara@linaro.org> wrote: > >> On 12/12/2013 10:23 AM, Peter Maydell wrote: > >>> What does it mean to say that a system register for AArch64 > >>> is "32 bits" given that MRS/MSR always operate on a 64 bit > >>> register? > > > >> But ARMv8 ARM still defines these registers as 32-bit: > >> D8.5.14: CNTV_CTL_EL0 > >> Attributes > >> CNTV_CTL_EL0 is a 32-bit register. > >> But indeed the MSR/MRS instruction references a Xt register, and the > >> documentation does not seem to tell how this is handled, so I assume this is > >> zero-extended. > > > > I checked, and for AArch64 registers, "32 bits" is just > > a shorthand for "64 bit register where the top 32 bits are > > RAZ/WI" (and I suspect it's not totally impossible that some > > future architecture revision might define new bits in the > > top half). > > Indeed. Actually, there isn't an instruction to access these 32bit > registers with a 'W' register. You really have to use a 'X'. > > > So I would suggest that we should make the KVM user<->kernel > > interface just consistently make all the sysregs 64 bit. > > > > (There is actually precedent of a sort here in that the > > kernel claims the PSTATE register is 64 bits wide despite > > it really being a 32 bit SPSR format value under the hood.) > > I definitely agree with Peter here. I'd like to keep the ABI 64bit for > the sysregs. It makes the whole thing much nicer. > OK, makes perfect sense, so we just need to change the patch to export the system registers as 64 bit in size, and then actually export them on arm64. -Christoffer
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index a30035d..9565e6a 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -1889,9 +1889,12 @@ value in the kvm_regs structure seen as a 32bit array. arm64 CCSIDR registers are demultiplexed by CSSELR value: 0x6020 0000 0011 00 <csselr:8> -arm64 system registers have the following id bit patterns: +arm64 64-bit system registers have the following id bit patterns: 0x6030 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> +arm64 32-bit system registers have the following id bit patterns: + 0x6020 0000 0013 <op0:2> <op1:3> <crn:4> <crm:4> <op2:3> + 4.69 KVM_GET_ONE_REG Capability: KVM_CAP_ONE_REG