| Message ID | 1597241133-3630-1-git-send-email-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm(64)/kvm: improve the documentation about HVC calls | expand |
This issue is detected by Morse (https://lore.kernel.org/linux-arm-kernel/9b0da257-785b-90ba-de3c-b9ee9ccdeeba@arm.com/) during the discussion for my patch. I am not quite sure about the arm, but based on the note at the head of arm/kernel/head.S, things should go that way. If any mistake, please correct me. Thanks, Pingfan On Wed, Aug 12, 2020 at 10:05 PM Pingfan Liu <kernelfans@gmail.com> wrote: > > Both arm and arm64 kernel entry point have the following prerequisite: > MMU = off, D-cache = off, I-cache = dont care. > > HVC_SOFT_RESTART call should meet this prerequisite before jumping to the > new kernel. > > Furthermore, on arm64, el2_setup doesn't set I+C bits and keeps EL2 MMU > off, and KVM resets them when its unload. These are achieved by > HVC_RESET_VECTORS call. > > Improve the document. > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > Cc: James Morse <james.morse@arm.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Julien Thierry <julien.thierry.kdev@gmail.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: linux-doc@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > To: kvmarm@lists.cs.columbia.edu > --- > Documentation/virt/kvm/arm/hyp-abi.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/virt/kvm/arm/hyp-abi.rst b/Documentation/virt/kvm/arm/hyp-abi.rst > index d9eba93..a95bc30 100644 > --- a/Documentation/virt/kvm/arm/hyp-abi.rst > +++ b/Documentation/virt/kvm/arm/hyp-abi.rst > @@ -40,9 +40,9 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > > * :: > > - r0/x0 = HVC_RESET_VECTORS > + x0 = HVC_RESET_VECTORS (arm64 only) > > - Turn HYP/EL2 MMU off, and reset HVBAR/VBAR_EL2 to the initials > + Disable HYP/EL2 MMU and D-cache, and reset HVBAR/VBAR_EL2 to the initials > stubs' exception vector value. This effectively disables an existing > hypervisor. > > @@ -54,7 +54,7 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > x3 = x1's value when entering the next payload (arm64) > x4 = x2's value when entering the next payload (arm64) > > - Mask all exceptions, disable the MMU, move the arguments into place > + Mask all exceptions, disable the MMU and D-cache, move the arguments into place > (arm64 only), and jump to the restart address while at HYP/EL2. This > hypercall is not expected to return to its caller. > > -- > 2.7.5 >
Hi Pingfan, On 12/08/2020 15:05, Pingfan Liu wrote: > Both arm and arm64 kernel entry point have the following prerequisite: > MMU = off, D-cache = off, I-cache = dont care. > > HVC_SOFT_RESTART call should meet this prerequisite before jumping to the > new kernel. I think you have this the wrong way up. This should describe what HVC_SOFT_RESTART does. We want to remove some extra work kexec does on arm64, and both implementations of HVC_SOFT_RESTART on arm64 already do what we need. The change here should be to document that the D/I bits are cleared after a HVC_SOFT_RESTART on arm64. > Furthermore, on arm64, el2_setup doesn't set I+C bits and keeps EL2 MMU > off, and KVM resets them when its unload. These are achieved by > HVC_RESET_VECTORS call. > > Improve the document. > diff --git a/Documentation/virt/kvm/arm/hyp-abi.rst b/Documentation/virt/kvm/arm/hyp-abi.rst > index d9eba93..a95bc30 100644 > --- a/Documentation/virt/kvm/arm/hyp-abi.rst > +++ b/Documentation/virt/kvm/arm/hyp-abi.rst > @@ -40,9 +40,9 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > > * :: > > - r0/x0 = HVC_RESET_VECTORS > + x0 = HVC_RESET_VECTORS (arm64 only) > > - Turn HYP/EL2 MMU off, and reset HVBAR/VBAR_EL2 to the initials > + Disable HYP/EL2 MMU and D-cache, and reset HVBAR/VBAR_EL2 to the initials > stubs' exception vector value. This effectively disables an existing > hypervisor. I don't think we should remove this. KVM on 32bit was the only implementer, but if there ever is another, this is how it should work. > @@ -54,7 +54,7 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > x3 = x1's value when entering the next payload (arm64) > x4 = x2's value when entering the next payload (arm64) > > - Mask all exceptions, disable the MMU, move the arguments into place > + Mask all exceptions, disable the MMU and D-cache, move the arguments into place > (arm64 only), and jump to the restart address while at HYP/EL2. This > hypercall is not expected to return to its caller. (I don't think disable the D-cache is what the bit does, it forces the attributes that are used for a data access). Please just describe this as the on arm64 the D and I bits are cleared. (it might be true on 32bit, I can't work the assembly out). Thanks, James
On Thu, Aug 27, 2020 at 2:10 AM James Morse <james.morse@arm.com> wrote: > > Hi Pingfan, > > On 12/08/2020 15:05, Pingfan Liu wrote: > > Both arm and arm64 kernel entry point have the following prerequisite: > > MMU = off, D-cache = off, I-cache = dont care. > > > > HVC_SOFT_RESTART call should meet this prerequisite before jumping to the > > new kernel. > > I think you have this the wrong way up. This should describe what HVC_SOFT_RESTART does. Yes, it is a wrong way. > > We want to remove some extra work kexec does on arm64, and both implementations of > HVC_SOFT_RESTART on arm64 already do what we need. The change here should be to document > that the D/I bits are cleared after a HVC_SOFT_RESTART on arm64. > > > > Furthermore, on arm64, el2_setup doesn't set I+C bits and keeps EL2 MMU > > off, and KVM resets them when its unload. These are achieved by > > HVC_RESET_VECTORS call. > > > > Improve the document. > > > > diff --git a/Documentation/virt/kvm/arm/hyp-abi.rst b/Documentation/virt/kvm/arm/hyp-abi.rst > > index d9eba93..a95bc30 100644 > > --- a/Documentation/virt/kvm/arm/hyp-abi.rst > > +++ b/Documentation/virt/kvm/arm/hyp-abi.rst > > @@ -40,9 +40,9 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > > > > * :: > > > > - r0/x0 = HVC_RESET_VECTORS > > + x0 = HVC_RESET_VECTORS (arm64 only) > > > > - Turn HYP/EL2 MMU off, and reset HVBAR/VBAR_EL2 to the initials > > + Disable HYP/EL2 MMU and D-cache, and reset HVBAR/VBAR_EL2 to the initials > > stubs' exception vector value. This effectively disables an existing > > hypervisor. > > I don't think we should remove this. KVM on 32bit was the only implementer, but if there > ever is another, this is how it should work. > > > > @@ -54,7 +54,7 @@ these functions (see arch/arm{,64}/include/asm/virt.h): > > x3 = x1's value when entering the next payload (arm64) > > x4 = x2's value when entering the next payload (arm64) > > > > - Mask all exceptions, disable the MMU, move the arguments into place > > + Mask all exceptions, disable the MMU and D-cache, move the arguments into place > > (arm64 only), and jump to the restart address while at HYP/EL2. This > > hypercall is not expected to return to its caller. > > (I don't think disable the D-cache is what the bit does, it forces the attributes that are > used for a data access). > > Please just describe this as the on arm64 the D and I bits are cleared. OK, I will do it. > (it might be true on 32bit, I can't work the assembly out). I will leave 32bit as it is. Thanks, Pingfan
diff --git a/Documentation/virt/kvm/arm/hyp-abi.rst b/Documentation/virt/kvm/arm/hyp-abi.rst index d9eba93..a95bc30 100644 --- a/Documentation/virt/kvm/arm/hyp-abi.rst +++ b/Documentation/virt/kvm/arm/hyp-abi.rst @@ -40,9 +40,9 @@ these functions (see arch/arm{,64}/include/asm/virt.h): * :: - r0/x0 = HVC_RESET_VECTORS + x0 = HVC_RESET_VECTORS (arm64 only) - Turn HYP/EL2 MMU off, and reset HVBAR/VBAR_EL2 to the initials + Disable HYP/EL2 MMU and D-cache, and reset HVBAR/VBAR_EL2 to the initials stubs' exception vector value. This effectively disables an existing hypervisor. @@ -54,7 +54,7 @@ these functions (see arch/arm{,64}/include/asm/virt.h): x3 = x1's value when entering the next payload (arm64) x4 = x2's value when entering the next payload (arm64) - Mask all exceptions, disable the MMU, move the arguments into place + Mask all exceptions, disable the MMU and D-cache, move the arguments into place (arm64 only), and jump to the restart address while at HYP/EL2. This hypercall is not expected to return to its caller.
Both arm and arm64 kernel entry point have the following prerequisite: MMU = off, D-cache = off, I-cache = dont care. HVC_SOFT_RESTART call should meet this prerequisite before jumping to the new kernel. Furthermore, on arm64, el2_setup doesn't set I+C bits and keeps EL2 MMU off, and KVM resets them when its unload. These are achieved by HVC_RESET_VECTORS call. Improve the document. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: James Morse <james.morse@arm.com> Cc: Russell King <linux@armlinux.org.uk> Cc: Marc Zyngier <maz@kernel.org> Cc: Julien Thierry <julien.thierry.kdev@gmail.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: linux-doc@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org To: kvmarm@lists.cs.columbia.edu --- Documentation/virt/kvm/arm/hyp-abi.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)