Re: [PATCH] KVM: arm/arm64: vgic-v3: Don't pretend to support IRQ/FIQ bypass

[Christoffer Dall]

On Wed, Feb 22, 2017 at 12:13:48PM +, Marc Zyngier wrote:
> Our GICv3 emulation always presents ICC_SRE_EL1 with DIB/DFB set to
> zero, which implies that there is a way to bypass the GIC and
> inject raw IRQ/FIQ by driving the CPU pins.
> 
> Of course, we don't allow that when the GIC is configured, but
> we fail to indicate that to the guest. The obvious fix is to
> set these bits (and never let them being changed again).
> 
> Reported-by: Peter Maydell 
> Signed-off-by: Marc Zyngier 

Acked-by: Christoffer Dall 

> ---
>  include/linux/irqchip/arm-gic-v3.h | 2 ++
>  virt/kvm/arm/vgic/vgic-v3.c| 5 -
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/irqchip/arm-gic-v3.h 
> b/include/linux/irqchip/arm-gic-v3.h
> index e808f8ae6f14..0a8bad331341 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -354,6 +354,8 @@
>   */
>  #define ICC_CTLR_EL1_EOImode_drop_dir(0U << 1)
>  #define ICC_CTLR_EL1_EOImode_drop(1U << 1)
> +#define ICC_SRE_EL1_DIB  (1U << 2)
> +#define ICC_SRE_EL1_DFB  (1U << 1)
>  #define ICC_SRE_EL1_SRE  (1U << 0)
>  
>  /*
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index e6b03fd8c374..d062256131fc 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -215,10 +215,13 @@ void vgic_v3_enable(struct kvm_vcpu *vcpu)
>   /*
>* If we are emulating a GICv3, we do it in an non-GICv2-compatible
>* way, so we force SRE to 1 to demonstrate this to the guest.
> +  * Also, we don't support any form of IRQ/FIQ bypass.
>* This goes with the spec allowing the value to be RAO/WI.
>*/
>   if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> - vgic_v3->vgic_sre = ICC_SRE_EL1_SRE;
> + vgic_v3->vgic_sre = (ICC_SRE_EL1_DIB |
> +  ICC_SRE_EL1_DFB |
> +  ICC_SRE_EL1_SRE);
>   vcpu->arch.vgic_cpu.pendbaser = INITIAL_PENDBASER_VALUE;
>   } else {
>   vgic_v3->vgic_sre = 0;
> -- 
> 2.11.0
> 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC PATCH 00/13] arm64/kvm: use common sysreg definitions

[Mark Rutland]

On Fri, Feb 24, 2017 at 11:16:50AM +0100, Christoffer Dall wrote:
> Hi Mark,
> 
> On Tue, Jan 31, 2017 at 06:05:38PM +, Mark Rutland wrote:
> > Whenever we add new functionality involving new system registers, we need to
> > add sys_reg() definitions so that we can access the registers regardless of
> > whether the toolchain can assemble them. At the same time, we have to add
> > duplicate definitions of the register encodings to KVM's sysreg tables, so 
> > that
> > we can handle any configurable traps. This redundancy is unfortunate, and
> > defining the encodings directly in the sysreg tables can make those tables
> > difficult to read.
> > 
> > This series attempts to address both of these issues by allowing us to use
> > common sys_reg() mnemonics in  to initialise KVM's sysreg 
> > tables.
> > To that end, this series tries to make  the canonical location
> > for common sysreg encodings.

> I did not do a full in-depth review, but I really like this overall
> change and the changes to KVM look great to me.

Cool; I'll respin+repost this once rc1's out.

I'll have to prepare a prize for whoever's willing to verify the
encodings. ;)

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: A question about TTBRs

[Mark Rutland]

On Fri, Feb 24, 2017 at 11:22:40AM +0100, Christoffer Dall wrote:
> On Fri, Feb 24, 2017 at 09:55:09AM +, Raz wrote:
> > Hello
> > I am reading the arm8a book. According to the documentation the output
> > address of each level 3 entry in TTBRx_EL1points to an address in the
> > physical memory.
> > By looking in the mmu tab in the DS5 studio I can see the TTBRs tables.
> > 
> > What I do not understand is why while I have 2GB of RAM in the FVP (
> > /proc/meminfo ) some page entries ( level 3 ) of the ttbr points to memory
> > above 4GB; for instance:
> > 
> > Output address NP:0xF794D000
> > 
> > Doesn't the physical memory starts at address zero ? if not, where its
> > starting point is configured?
> 
> It depends on your particular system where RAM starts, and it does not
> necessarily start at zero.  You'd have to check the documentation of
> your model or hardware or look at the device tree you use, for example.

It's also worth bearing in mind that memory is not necessarily
physically contiguous. There may be several banks with gaps in the
middle, as is the case on ARM Juno systems [1]:

memory@8000 {
device_type = "memory";
/* last 16MB of the first memory area is reserved for secure 
world use by firmware */
reg = <0x 0x8000 0x0 0x7f00>,
  <0x0008 0x8000 0x1 0x8000>;
};

It may also be the case that MMIO devices fall within these gaps.

Thanks,
Mark.

[1] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/arm/juno-base.dtsi?h=v4.10=c470abd4fde40ea6a0846a2beab642a578c0b8cd
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support

[James Morse]

Hi Tyler,

On 21/02/17 21:22, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.

> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT(0x04)
>  #define FSC_ACCESS   (0x08)
>  #define FSC_PERM (0x0c)
> +#define FSC_EXTABT   (0x10)

arm64 has ESR_ELx_FSC_EXTABT which is used in inject_abt64(), but for matching
an external abort coming from hardware the range is wider.

Looking at the ARM-ARMs 'ISS encoding for an exception from an Instruction
Abort' in 'D7.2.27 ESR_ELx, Exception Syndrome Register (ELx)' (page D7-1954 of
version 'k'...iss10775), the ten flavours of you Synchronous abort you hooked
with do_sea() in patch 4 occupy 0x10 to 0x1f...


> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..04f1dd50 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "trace.h"
>  
> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, 
> struct kvm_run *run)
>  
>   /* Check the stage-2 fault is trans. fault or write fault */
>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);

... kvm_vcpu_trap_get_fault_type() on both arm and arm64 masks the HSR/ESR_EL2
with 0x3c ...


> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> +  * is no need to pass the error into the guest.
> +  */
> + if (fault_status == FSC_EXTABT) {

... but here we only check for 'Synchronous external abort, not on a translation
table walk'. Are the other types relevant?

If so we need some helper as this range is sparse and 'all other values are
reserved'. The aarch32 HSR format is slightly different. (G6-4411 ISS encoding
from an exception from a Data Abort).

If not, can we change patch 4 to check this type too so we don't call out to
APEI for a fault type we know isn't relevant.


> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {
> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>   kvm_vcpu_trap_get_class(vcpu),
>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..403277b 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{

> + if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
> + nmi_enter();
> + ghes_notify_sea();
> + nmi_exit();

This nmi stuff was needed for synchronous aborts that may have interrupted
APEI's interrupts-masked code. We want to avoid trying to take the same set of
locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
a guest, so there is no risk that we have interrupted APEI on the host.
ghes_notify_sea() can safely take the normal path.


Thanks,

James
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/55] Nested Virtualization on KVM/ARM

[Jintack Lim]

[My previous reply had HTML subpart, which made the e-mail look
terrible and being rejected from mailing lists. So, I'm sending it
again. Sorry for the inconvenience]

Hi Christoffer,

On Wed, Feb 22, 2017 at 1:23 PM, Christoffer Dall  wrote:
> Hi Jintack,
>
>
> On Mon, Jan 09, 2017 at 01:23:56AM -0500, Jintack Lim wrote:
>> Nested virtualization is the ability to run a virtual machine inside another
>> virtual machine. In other words, it’s about running a hypervisor (the guest
>> hypervisor) on top of another hypervisor (the host hypervisor).
>>
>> This series supports nested virtualization on arm64. ARM recently announced 
>> an
>> extension (ARMv8.3) which has support for nested virtualization[1]. This 
>> series
>> is based on the ARMv8.3 specification.
>>
>> Supporting nested virtualization means that the hypervisor provides not only
>> EL0/EL1 execution environment with VMs as it usually does, but also the
>> virtualization extensions including EL2 execution environment with the VMs.
>> Once the host hypervisor provides those execution environment with the VMs,
>> then the guest hypervisor can run its own VMs (nested VMs) naturally.
>>
>> To support nested virtualization on ARM the hypervisor must emulate a virtual
>> execution environment consisting of EL2, EL1, and EL0, as the guest 
>> hypervisor
>> will run in a virtual EL2 mode.  Normally KVM/ARM only emulated a VM 
>> supporting
>> EL1/0 running in their respective native CPU modes, but with nested
>> virtualization we deprivilege the guest hypervisor and emulate a virtual EL2
>> execution mode in EL1 using the hardware features provided by ARMv8.3 to trap
>> EL2 operations to EL1. To do that the host hypervisor needs to manage EL2
>> register state for the guest hypervisor, and shadow EL1 register state that
>> reflects the EL2 register state to run the guest hypervisor in EL1. See 
>> patch 6
>> through 10 for this.
>>
>> For memory virtualization, the biggest issue is that we now have more than 
>> two
>> stages of translation when running nested VMs. We choose to merge two stage-2
>> page tables (one from the guest hypervisor and the other from the host
>> hypervisor) and create shadow stage-2 page tables, which have mappings from 
>> the
>> nested VM’s physical addresses to the machine physical addresses. Stage-1
>> translation is done by the hardware as is done for the normal VMs.
>>
>> To provide VGIC support to the guest hypervisor, we emulate the GIC
>> virtualization extensions using trap-and-emulate to a virtual GIC Hypervisor
>> Control Interface.  Furthermore, we can still use the GIC VE hardware 
>> features
>> to deliver virtual interrupts to the nested VM, by directly mapping the GIC
>> VCPU interface to the nested VM and switching the content of the GIC 
>> Hypervisor
>> Control interface when alternating between a nested VM and a normal VM.  See
>> patches 25 through 32, and 50 through 52 for more information.
>>
>> For timer virtualization, the guest hypervisor expects to have access to the
>> EL2 physical timer, the EL1 physical timer and the virtual timer. So, the 
>> host
>> hypervisor needs to provide all of them. The virtual timer is always 
>> available
>> to VMs. The physical timer is available to VMs via my previous patch 
>> series[3].
>> The EL2 physical timer is not supported yet in this RFC. We plan to support
>> this as it is required to run other guest hypervisors such as Xen.
>>
>> Even though this work is not complete (see limitations below), I'd appreciate
>> early feedback on this RFC. Specifically, I'm interested in:
>> - Is it better to have a kernel config or to make it configurable at runtime?
>> - I wonder if the data structure for memory management makes sense.
>> - What architecture version do we support for the guest hypervisor, and how?
>>   For example, do we always support all architecture versions or the same
>>   architecture as the underlying hardware platform? Or is it better
>>   to make it configurable from the userspace?
>> - Initial comments on the overall design?
>>
>> This patch series is based on kvm-arm-for-4.9-rc7 with the patch series to 
>> provide
>> VMs with the EL1 physical timer[2].
>>
>> Git: https://github.com/columbia/nesting-pub/tree/rfc-v1
>>
>> Testing:
>> We have tested this on ARMv8.0 (Applied Micro X-Gene)[3] since ARMv8.3 
>> hardware
>> is not available yet. We have paravirtualized the guest hypervisor to trap to
>> EL2 as specified in ARMv8.3 specification using hvc instruction. We plan to
>> test this on ARMv8.3 model, and will post the result and v2 if necessary.
>>
>> Limitations:
>> - This patch series only supports arm64, not arm. All the patches compile on
>>   arm, but I haven't try to boot normal VMs on it.
>> - The guest hypervisor with VHE (ARMv8.1) is not supported in this RFC. I 
>> have
>>   patches for that, but they need to be cleaned up.
>> - Recursive nesting (i.e. emulating ARMv8.3 in the VM) is not tested yet.
>> - Other 

Re: A question about TTBRs

[Christoffer Dall]

On Fri, Feb 24, 2017 at 09:55:09AM +, Raz wrote:
> Hello
> I am reading the arm8a book. According to the documentation the output
> address of each level 3 entry in TTBRx_EL1points to an address in the
> physical memory.
> By looking in the mmu tab in the DS5 studio I can see the TTBRs tables.
> 
> What I do not understand is why while I have 2GB of RAM in the FVP (
> /proc/meminfo ) some page entries ( level 3 ) of the ttbr points to memory
> above 4GB; for instance:
> 
> Output address NP:0xF794D000
> 
> Doesn't the physical memory starts at address zero ? if not, where its
> starting point is configured?

It depends on your particular system where RAM starts, and it does not
necessarily start at zero.  You'd have to check the documentation of
your model or hardware or look at the device tree you use, for example.

-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC PATCH 00/13] arm64/kvm: use common sysreg definitions

[Christoffer Dall]

Hi Mark,

On Tue, Jan 31, 2017 at 06:05:38PM +, Mark Rutland wrote:
> Whenever we add new functionality involving new system registers, we need to
> add sys_reg() definitions so that we can access the registers regardless of
> whether the toolchain can assemble them. At the same time, we have to add
> duplicate definitions of the register encodings to KVM's sysreg tables, so 
> that
> we can handle any configurable traps. This redundancy is unfortunate, and
> defining the encodings directly in the sysreg tables can make those tables
> difficult to read.
> 
> This series attempts to address both of these issues by allowing us to use
> common sys_reg() mnemonics in  to initialise KVM's sysreg 
> tables.
> To that end, this series tries to make  the canonical location
> for common sysreg encodings.
> 
> Largely, I've only attacked the AArch64-native SYS encodings required by KVM
> today, though for the debug and perfmon groups it was easier to take the whole
> group from the ARM ARM than to filter them to only what KVM needed. I've
> ignored CP{15,14} registers for now, but these could be encoded similarly.
> 
> To verify that I haven't accidentally broken KVM, I've diffed sys_regs.o and
> sys_regs_generic_v8.o on a section-by-section basis before and after the 
> series
> is applied. The .text, .data, and .rodata sections (and most others) are
> identical. The __bug_table section, and some .debug* sections differ, and this
> appears to be due to line numbers changing due to removed lines.
> 
> One thing I wasn't sure how to address was banks of registers such as
> PMEVCNTR_EL0. We currently enumerate all cases for our GICv3 definitions,
> but it seemed painful to expand ~30 cases for PMEVCNTR_EL0 and friends, and
> for these I've made the macros take an 'n' parameter.
> 
> The series is based on the arm64/for-next/core branch, since it relies on
> commit c9ee0f98662a6e35 ("arm64: cpufeature: Define helpers for sys_reg id")
> for the definition of SYS_DESC().
> 

I did not do a full in-depth review, but I really like this overall
change and the changes to KVM look great to me.

Thanks for doing this!
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/55] Nested Virtualization on KVM/ARM

[Jintack Lim]

Hi Christoffer,

On Wed, Feb 22, 2017 at 1:23 PM, Christoffer Dall  wrote:

> Hi Jintack,
>
>
> On Mon, Jan 09, 2017 at 01:23:56AM -0500, Jintack Lim wrote:
> > Nested virtualization is the ability to run a virtual machine inside
> another
> > virtual machine. In other words, it’s about running a hypervisor (the
> guest
> > hypervisor) on top of another hypervisor (the host hypervisor).
> >
> > This series supports nested virtualization on arm64. ARM recently
> announced an
> > extension (ARMv8.3) which has support for nested virtualization[1]. This
> series
> > is based on the ARMv8.3 specification.
> >
> > Supporting nested virtualization means that the hypervisor provides not
> only
> > EL0/EL1 execution environment with VMs as it usually does, but also the
> > virtualization extensions including EL2 execution environment with the
> VMs.
> > Once the host hypervisor provides those execution environment with the
> VMs,
> > then the guest hypervisor can run its own VMs (nested VMs) naturally.
> >
> > To support nested virtualization on ARM the hypervisor must emulate a
> virtual
> > execution environment consisting of EL2, EL1, and EL0, as the guest
> hypervisor
> > will run in a virtual EL2 mode.  Normally KVM/ARM only emulated a VM
> supporting
> > EL1/0 running in their respective native CPU modes, but with nested
> > virtualization we deprivilege the guest hypervisor and emulate a virtual
> EL2
> > execution mode in EL1 using the hardware features provided by ARMv8.3 to
> trap
> > EL2 operations to EL1. To do that the host hypervisor needs to manage EL2
> > register state for the guest hypervisor, and shadow EL1 register state
> that
> > reflects the EL2 register state to run the guest hypervisor in EL1. See
> patch 6
> > through 10 for this.
> >
> > For memory virtualization, the biggest issue is that we now have more
> than two
> > stages of translation when running nested VMs. We choose to merge two
> stage-2
> > page tables (one from the guest hypervisor and the other from the host
> > hypervisor) and create shadow stage-2 page tables, which have mappings
> from the
> > nested VM’s physical addresses to the machine physical addresses. Stage-1
> > translation is done by the hardware as is done for the normal VMs.
> >
> > To provide VGIC support to the guest hypervisor, we emulate the GIC
> > virtualization extensions using trap-and-emulate to a virtual GIC
> Hypervisor
> > Control Interface.  Furthermore, we can still use the GIC VE hardware
> features
> > to deliver virtual interrupts to the nested VM, by directly mapping the
> GIC
> > VCPU interface to the nested VM and switching the content of the GIC
> Hypervisor
> > Control interface when alternating between a nested VM and a normal VM.
> See
> > patches 25 through 32, and 50 through 52 for more information.
> >
> > For timer virtualization, the guest hypervisor expects to have access to
> the
> > EL2 physical timer, the EL1 physical timer and the virtual timer. So,
> the host
> > hypervisor needs to provide all of them. The virtual timer is always
> available
> > to VMs. The physical timer is available to VMs via my previous patch
> series[3].
> > The EL2 physical timer is not supported yet in this RFC. We plan to
> support
> > this as it is required to run other guest hypervisors such as Xen.
> >
> > Even though this work is not complete (see limitations below), I'd
> appreciate
> > early feedback on this RFC. Specifically, I'm interested in:
> > - Is it better to have a kernel config or to make it configurable at
> runtime?
> > - I wonder if the data structure for memory management makes sense.
> > - What architecture version do we support for the guest hypervisor, and
> how?
> >   For example, do we always support all architecture versions or the same
> >   architecture as the underlying hardware platform? Or is it better
> >   to make it configurable from the userspace?
> > - Initial comments on the overall design?
> >
> > This patch series is based on kvm-arm-for-4.9-rc7 with the patch series
> to provide
> > VMs with the EL1 physical timer[2].
> >
> > Git: https://github.com/columbia/nesting-pub/tree/rfc-v1
> >
> > Testing:
> > We have tested this on ARMv8.0 (Applied Micro X-Gene)[3] since ARMv8.3
> hardware
> > is not available yet. We have paravirtualized the guest hypervisor to
> trap to
> > EL2 as specified in ARMv8.3 specification using hvc instruction. We plan
> to
> > test this on ARMv8.3 model, and will post the result and v2 if necessary.
> >
> > Limitations:
> > - This patch series only supports arm64, not arm. All the patches
> compile on
> >   arm, but I haven't try to boot normal VMs on it.
> > - The guest hypervisor with VHE (ARMv8.1) is not supported in this RFC.
> I have
> >   patches for that, but they need to be cleaned up.
> > - Recursive nesting (i.e. emulating ARMv8.3 in the VM) is not tested yet.
> > - Other hypervisors (such as Xen) on KVM are not tested.
> >
> > TODO:
> > - Test to boot normal 

A question about TTBRs

[Raz]

Hello
I am reading the arm8a book. According to the documentation the output
address of each level 3 entry in TTBRx_EL1points to an address in the
physical memory.
By looking in the mmu tab in the DS5 studio I can see the TTBRs tables.

What I do not understand is why while I have 2GB of RAM in the FVP (
/proc/meminfo ) some page entries ( level 3 ) of the ttbr points to memory
above 4GB; for instance:

Output address NP:0xF794D000

Doesn't the physical memory starts at address zero ? if not, where its
starting point is configured?

Thank you
Raz
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm