Re: [PATCH v2 1/5] hw/arm/virt: Key enablement of highmem PCIe on highmem_ecam

2022-01-04 Thread Marc Zyngier
Hi Eric,

On Tue, 04 Jan 2022 15:31:33 +,
Eric Auger  wrote:
> 
> Hi Marc,
> 
> On 12/27/21 4:53 PM, Marc Zyngier wrote:
> > Hi Eric,
> >
> > Picking this up again after a stupidly long time...
> >
> > On Mon, 04 Oct 2021 13:00:21 +0100,
> > Eric Auger  wrote:
> >> Hi Marc,
> >>
> >> On 10/3/21 6:46 PM, Marc Zyngier wrote:
> >>> Currently, the highmem PCIe region is oddly keyed on the highmem
> >>> attribute instead of highmem_ecam. Move the enablement of this PCIe
> >>> region over to highmem_ecam.
> >>>
> >>> Signed-off-by: Marc Zyngier 
> >>> ---
> >>>  hw/arm/virt-acpi-build.c | 10 --
> >>>  hw/arm/virt.c|  4 ++--
> >>>  2 files changed, 6 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >>> index 037cc1fd82..d7bef0e627 100644
> >>> --- a/hw/arm/virt-acpi-build.c
> >>> +++ b/hw/arm/virt-acpi-build.c
> >>> @@ -157,10 +157,9 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> >>>  }
> >>>  
> >>>  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> >>> -  uint32_t irq, bool use_highmem, bool 
> >>> highmem_ecam,
> >>> -  VirtMachineState *vms)
> >>> +  uint32_t irq, VirtMachineState *vms)
> >>>  {
> >>> -int ecam_id = VIRT_ECAM_ID(highmem_ecam);
> >>> +int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
> >>>  struct GPEXConfig cfg = {
> >>>  .mmio32 = memmap[VIRT_PCIE_MMIO],
> >>>  .pio= memmap[VIRT_PCIE_PIO],
> >>> @@ -169,7 +168,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
> >>> MemMapEntry *memmap,
> >>>  .bus= vms->bus,
> >>>  };
> >>>  
> >>> -if (use_highmem) {
> >>> +if (vms->highmem_ecam) {
> >> highmem_ecam is more restrictive than use_highmem:
> >> vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
> >>
> >> If I remember correctly there was a problem using highmem ECAM with 32b
> >> AAVMF FW.
> >>
> >> However 5125f9cd2532 ("hw/arm/virt: Add high MMIO PCI region, 512G in
> >> size") introduced high MMIO PCI region without this constraint.
> > Then I really don't understand the point of this highmem_ecam. We only
> > register the highmem version if highmem_ecam is set (see the use of
> > VIRT_ECAM_ID() to pick the right ECAM window).
> 
> but aren't we talking about different regions? On one hand the [high]
> MMIO region (512GB wide) and the [high] ECAM region (256MB large).
> To me you can enable either independently. High MMIO region is used by
> some devices likes ivshmem/video cards while high ECAM was introduced to
> extend the number of supported buses: 601d626d148a (hw/arm/virt: Add a
> new 256MB ECAM region).
> 
> with the above change the high MMIO region won't be set with 32b
> FW+kernel and LPAE whereas it is currently.
> 
> high ECAM was not supported by 32b FW, hence the highmem_ecam.
> 
> but maybe I miss your point?

There are two issues.

First, I have been conflating the ECAM and MMIO ranges, and you only
made me realise that they were supposed to be independent.  I still
think the keying on highmem is wrong, but the main issue is that the
highmem* flags don't quite describe the shape of the platform.

All these booleans indicate is whether the feature they describe (the
high MMIO range, the high ECAM range, and in one of my patches the
high RD range) are *allowed* to live above 4GB, but do not express
whether then are actually usable (i.e. fit in the PA range).

Maybe we need to be more thorough in the way we describe the extended
region in the VirtMachineState structure:

- highmem: overall control for anything that *can* live above 4GB
- highmem_ecam: Has a PCIe ECAM region above 256GB
- highmem_mmio: Has a PCIe MMIO region above 256GB
- highmem_redist: Has 512 RDs above 256GB

Crucially, the last 3 items must fit in the PA range or be disabled.

We have highmem_ecam which is keyed on highmem, but not on the PA
range.  highmem_mmio doesn't exist at all (we use highmem instead),
and I'm only introducing highmem_redist.

For these 3 ranges, we should have something like

vms->highmem_xxx &= (vms->highmem &&
 (vms->memmap[XXX].base + vms->vms->memmap[XXX].size) < 
vms->highest_gpa);

and treat them as independent entities.  Unless someone shouts, I'm
going to go ahead and implement this logic.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 1/1] KVM: arm64: vgic: Replace kernel.h with the necessary inclusions

2022-01-04 Thread Marc Zyngier
On Tue, 4 Jan 2022 17:19:40 +0200, Andy Shevchenko wrote:
> The arm_vgic.h does not require all the stuff the kernel.h provides.
> Replace kernel.h inclusion with the list of what is really being used.

Applied to next, thanks!

[1/1] KVM: arm64: vgic: Replace kernel.h with the necessary inclusions
  commit: 6c9eeb5f4a9bb2b11a40fd0f15efde7bd33ee908

Cheers,

M.
-- 
Without deviation from the norm, progress is not possible.


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


Re: Possible nohz-full/RCU issue in arm64 KVM

2022-01-04 Thread Paolo Bonzini

On 1/4/22 17:39, Mark Rutland wrote:

My main issue here was just that it's really difficult to see how the
entry/exit logic is balanced, and I reckon we can solve that by splitting
guest_{enter,exit}_irqoff() into helper functions to handle the vtime
accounting separately from the context tracking, so that arch code can do
something like:

   guest_timing_enter_irqoff();
   
   guest_eqs_enter_irqoff();

   < actually run vCPU here >
   guest_eqs_exit_irqoff();
   
   < handle pending IRQs here >
   
   guest_timing_exit_irqoff();


... which I hope should work for RISC-V too.

I've had a go, and I've pushed out a WIP to:

   
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/kvm/rcu


Yes, you have a point and it makes sense for x86 too.  You can send me a 
topic branch once you get all the acks.  Thanks!


Paolo

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


[PATCH v2 1/1] KVM: arm64: vgic: Replace kernel.h with the necessary inclusions

2022-01-04 Thread Andy Shevchenko
The arm_vgic.h does not require all the stuff the kernel.h provides.
Replace kernel.h inclusion with the list of what is really being used.

Signed-off-by: Andy Shevchenko 
---
v2: updated commit message by dropping unrelated paragraph (Marc)
 include/kvm/arm_vgic.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index e602d848fc1a..bb30a6803d9f 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -5,9 +5,11 @@
 #ifndef __KVM_ARM_VGIC_H
 #define __KVM_ARM_VGIC_H
 
-#include 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.34.1

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


Re: Possible nohz-full/RCU issue in arm64 KVM

2022-01-04 Thread Mark Rutland
On Fri, Dec 17, 2021 at 04:54:22PM +0100, Paolo Bonzini wrote:
> On 12/17/21 15:38, Mark Rutland wrote:
> > For example kvm_guest_enter_irqoff() calls guest_enter_irq_off() which calls
> > vtime_account_guest_enter(), but kvm_guest_exit_irqoff() doesn't call
> > guest_exit_irq_off() and the call to vtime_account_guest_exit() is 
> > open-coded
> > elsewhere. Also, guest_enter_irq_off() conditionally calls
> > rcu_virt_note_context_switch(), but I can't immediately spot anything on the
> > exit side that corresponded with that, which looks suspicious.
> 
> rcu_note_context_switch() is a point-in-time notification; it's not strictly
> necessary, but it may improve performance a bit by avoiding unnecessary IPIs
> from the RCU subsystem.
> 
> There's no benefit from doing it when you're back from the guest, because at
> that point the CPU is just running normal kernel code.

I see.

My main issue here was just that it's really difficult to see how the
entry/exit logic is balanced, and I reckon we can solve that by splitting
guest_{enter,exit}_irqoff() into helper functions to handle the vtime
accounting separately from the context tracking, so that arch code can do
something like:

  guest_timing_enter_irqoff();
  
  guest_eqs_enter_irqoff();
  < actually run vCPU here >
  guest_eqs_exit_irqoff();
  
  < handle pending IRQs here >
  
  guest_timing_exit_irqoff();

... which I hope should work for RISC-V too.

I've had a go, and I've pushed out a WIP to:

  
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/kvm/rcu

I also see we'll need to add some lockdep/irq-tracing management to arm64, and
it probably makes sense to fold that into common helpers, so I'll have a play
with that tomorrow.

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


Re: [PATCH v2 1/5] hw/arm/virt: Key enablement of highmem PCIe on highmem_ecam

2022-01-04 Thread Eric Auger
Hi Marc,

On 12/27/21 4:53 PM, Marc Zyngier wrote:
> Hi Eric,
>
> Picking this up again after a stupidly long time...
>
> On Mon, 04 Oct 2021 13:00:21 +0100,
> Eric Auger  wrote:
>> Hi Marc,
>>
>> On 10/3/21 6:46 PM, Marc Zyngier wrote:
>>> Currently, the highmem PCIe region is oddly keyed on the highmem
>>> attribute instead of highmem_ecam. Move the enablement of this PCIe
>>> region over to highmem_ecam.
>>>
>>> Signed-off-by: Marc Zyngier 
>>> ---
>>>  hw/arm/virt-acpi-build.c | 10 --
>>>  hw/arm/virt.c|  4 ++--
>>>  2 files changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>> index 037cc1fd82..d7bef0e627 100644
>>> --- a/hw/arm/virt-acpi-build.c
>>> +++ b/hw/arm/virt-acpi-build.c
>>> @@ -157,10 +157,9 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>>>  }
>>>  
>>>  static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>>> -  uint32_t irq, bool use_highmem, bool 
>>> highmem_ecam,
>>> -  VirtMachineState *vms)
>>> +  uint32_t irq, VirtMachineState *vms)
>>>  {
>>> -int ecam_id = VIRT_ECAM_ID(highmem_ecam);
>>> +int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
>>>  struct GPEXConfig cfg = {
>>>  .mmio32 = memmap[VIRT_PCIE_MMIO],
>>>  .pio= memmap[VIRT_PCIE_PIO],
>>> @@ -169,7 +168,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
>>> MemMapEntry *memmap,
>>>  .bus= vms->bus,
>>>  };
>>>  
>>> -if (use_highmem) {
>>> +if (vms->highmem_ecam) {
>> highmem_ecam is more restrictive than use_highmem:
>> vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
>>
>> If I remember correctly there was a problem using highmem ECAM with 32b
>> AAVMF FW.
>>
>> However 5125f9cd2532 ("hw/arm/virt: Add high MMIO PCI region, 512G in
>> size") introduced high MMIO PCI region without this constraint.
> Then I really don't understand the point of this highmem_ecam. We only
> register the highmem version if highmem_ecam is set (see the use of
> VIRT_ECAM_ID() to pick the right ECAM window).

but aren't we talking about different regions? On one hand the [high]
MMIO region (512GB wide) and the [high] ECAM region (256MB large).
To me you can enable either independently. High MMIO region is used by
some devices likes ivshmem/video cards while high ECAM was introduced to
extend the number of supported buses: 601d626d148a (hw/arm/virt: Add a
new 256MB ECAM region).

with the above change the high MMIO region won't be set with 32b
FW+kernel and LPAE whereas it is currently.

high ECAM was not supported by 32b FW, hence the highmem_ecam.

but maybe I miss your point?

Eric
>
> So keying this on highmem makes it expose a device that may not be
> there the first place since, as you pointed out that highmem_ecam can
> be false in cases where highmem is true.
>
>> So to me we should keep vms->highmem here
> I really must be missing how this is supposed to work.
>
>   M.
>

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


Re: [PATCH kvmtool 9/9] arm64: Add support for KVM_ARM_VCPU_PMU_V3_SET_PMU

2022-01-04 Thread Marc Zyngier
On Mon, 15 Nov 2021 16:57:05 +,
Alexandru Elisei  wrote:
> 
> The KVM_ARM_VCPU_PMU_V3_CTRL(KVM_ARM_VCPU_PMU_V3_SET_PMU) VCPU ioctl is
> used to assign a physical PMU to the events that KVM creates when emulating
> the PMU for that VCPU. This is useful on heterogeneous systems, when there
> is more than one hardware PMU present.
> 
> The assumption that is made in the implementation is that the user will
> pin the kvmtool process on a set of CPUs that share the same PMU. This
> allows kvmtool to set the same PMU for all VCPUs from the main thread,
> instead of in the individual VCPU threads.

May I suggest a slightly different use model? Ideally, you'd be able
to run the vcpu threads on the CPUs matching the PMU affinity, and
leave all the other threads to roam on other CPUs.

With your implementation, the whole of kvmtool gets stuck to a given
CPU type, which can be problematic.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 1/1] KVM: arm64: vgic: Replace kernel.h with the necessary inclusions

2022-01-04 Thread Andy Shevchenko
On Tue, Jan 04, 2022 at 01:44:31PM +, Marc Zyngier wrote:
> On Wed, 22 Dec 2021 19:14:28 +,
> Andy Shevchenko  wrote:
> > On Wed, Dec 22, 2021 at 08:25:43PM +0200, Andy Shevchenko wrote:
> > > On Wed, Dec 22, 2021 at 06:09:22PM +, Marc Zyngier wrote:
> > > > On Wed, 22 Dec 2021 16:55:52 +,
> > > > Andy Shevchenko  wrote:
> > > > > 
> > > > > When kernel.h is used in the headers it adds a lot into dependency 
> > > > > hell,
> > > > > especially when there are circular dependencies are involved.
> > > > 
> > > > Which circular dependencies? What problem are you solving?
> > > 
> > > In particular moving bitmap_*alloc() APIs to the headers.
> > > 
> > > But this may be a side effect of what I realized during the attempts
> > > of solving that issue. In any case there is no need to take entire
> > > mess of kernel.h in another header.
> > 
> > For the record  `make headerdep` doesn't make any difference with
> > or without this patch. But I consider it's better not to use kernel.h
> > in the headers due to a full mess behind it.
> 
> Can you then please write a commit message that matches what this is
> actually doing instead of mentioning a problem that doesn't seem to
> exist?

Sure, thanks for review!

-- 
With Best Regards,
Andy Shevchenko


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


Re: [PATCH] KVM: arm64: Fix comment typo in kvm_vcpu_finalize_sve()

2022-01-04 Thread Marc Zyngier
On Thu, 30 Dec 2021 22:15:35 +0800, Zenghui Yu wrote:
> kvm_arm_init_arch_resources() was renamed to kvm_arm_init_sve() in
> commit a3be836df7cb ("KVM: arm/arm64: Demote
> kvm_arm_init_arch_resources() to just set up SVE"). Fix the function
> name in comment of kvm_vcpu_finalize_sve().

Applied to next, thanks!

[1/1] KVM: arm64: Fix comment typo in kvm_vcpu_finalize_sve()
  commit: e938eddbeb85f4c0c47e56cd9e09ee196ea1bc1a

Cheers,

M.
-- 
Without deviation from the norm, progress is not possible.


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


Re: [PATCH v1 1/1] KVM: arm64: vgic: Replace kernel.h with the necessary inclusions

2022-01-04 Thread Marc Zyngier
On Wed, 22 Dec 2021 19:14:28 +,
Andy Shevchenko  wrote:
> 
> On Wed, Dec 22, 2021 at 08:25:43PM +0200, Andy Shevchenko wrote:
> > On Wed, Dec 22, 2021 at 06:09:22PM +, Marc Zyngier wrote:
> > > On Wed, 22 Dec 2021 16:55:52 +,
> > > Andy Shevchenko  wrote:
> > > > 
> > > > When kernel.h is used in the headers it adds a lot into dependency hell,
> > > > especially when there are circular dependencies are involved.
> > > 
> > > Which circular dependencies? What problem are you solving?
> > 
> > In particular moving bitmap_*alloc() APIs to the headers.
> > 
> > But this may be a side effect of what I realized during the attempts
> > of solving that issue. In any case there is no need to take entire
> > mess of kernel.h in another header.
> 
> For the record  `make headerdep` doesn't make any difference with
> or without this patch. But I consider it's better not to use kernel.h
> in the headers due to a full mess behind it.

Can you then please write a commit message that matches what this is
actually doing instead of mentioning a problem that doesn't seem to
exist?

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: Possible nohz-full/RCU issue in arm64 KVM

2022-01-04 Thread Mark Rutland
On Mon, Dec 20, 2021 at 05:10:14PM +0100, Frederic Weisbecker wrote:
> On Fri, Dec 17, 2021 at 01:21:39PM +, Mark Rutland wrote:
> > On Fri, Dec 17, 2021 at 12:51:57PM +0100, Nicolas Saenz Julienne wrote:
> > > Hi All,
> > 
> > Hi,
> > 
> > > arm64's guest entry code does the following:
> > > 
> > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > > {
> > >   [...]
> > > 
> > >   guest_enter_irqoff();
> > > 
> > >   ret = kvm_call_hyp_ret(__kvm_vcpu_run, vcpu);
> > > 
> > >   [...]
> > > 
> > >   local_irq_enable();
> > > 
> > >   /*
> > >* We do local_irq_enable() before calling guest_exit() so
> > >* that if a timer interrupt hits while running the guest we
> > >* account that tick as being spent in the guest.  We enable
> > >* preemption after calling guest_exit() so that if we get
> > >* preempted we make sure ticks after that is not counted as
> > >* guest time.
> > >*/
> > >   guest_exit();
> > >   [...]
> > > }
> > > 
> > > 
> > > On a nohz-full CPU, guest_{enter,exit}() delimit an RCU extended quiescent
> > > state (EQS). Any interrupt happening between local_irq_enable() and
> > > guest_exit() should disable that EQS. Now, AFAICT all el0 interrupt 
> > > handlers
> > > do the right thing if trggered in this context, but el1's won't. Is it
> > > possible to hit an el1 handler (for example __el1_irq()) there?
> > 
> > I think you're right that the EL1 handlers can trigger here and won't exit 
> > the
> > EQS.
> > 
> > I'm not immediately sure what we *should* do here. What does x86 do for an 
> > IRQ
> > taken from a guest mode? I couldn't spot any handling of that case, but I'm 
> > not
> > familiar enough with the x86 exception model to know if I'm looking in the
> > right place.
> 
> This is one of the purposes of rcu_irq_enter(). el1 handlers don't call 
> irq_enter()?

Due to lockep/tracing/etc ordering, we don't use irq_enter() directly and
instead call rcu_irq_enter() and irq_enter_rcu() separately. Critically we only
call rcu_irq_enter() for IRQs taken from the idle thread, as this was
previously thought to be the only place where we could take an IRQ from an EL1
EQS.

See __el1_irq(), __enter_from_kernel_mode(), and __exit_to_kernel_mode() in
arch/arm64/kernel/entry-common.c. The latter two are largely analogous to the
common irqentry_enter9) and irqentry_exit() helpers in kernel/entry/common.c.

We need to either rework the KVM code or that entry code. I'll dig into this a
bit more...

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


Re: [PATCH v5 34/69] KVM: arm64: nv: Configure HCR_EL2 for nested virtualization

2022-01-04 Thread Marc Zyngier
On Tue, 04 Jan 2022 08:53:42 +,
Ganapatrao Kulkarni  wrote:
> 
> 
> 
> On 30-11-2021 01:31 am, Marc Zyngier wrote:
> > From: Jintack Lim 
> > 
> > We enable nested virtualization by setting the HCR NV and NV1 bit.
> > 
> > When the virtual E2H bit is set, we can support EL2 register accesses
> > via EL1 registers from the virtual EL2 by doing trap-and-emulate. A
> > better alternative, however, is to allow the virtual EL2 to access EL2
> > register states without trap. This can be easily achieved by not traping
> > EL1 registers since those registers already have EL2 register states.
> > 
> > Signed-off-by: Jintack Lim 
> > Signed-off-by: Marc Zyngier 
> > ---
> >   arch/arm64/include/asm/kvm_arm.h |  1 +
> >   arch/arm64/kvm/hyp/vhe/switch.c  | 38 +---
> >   2 files changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_arm.h 
> > b/arch/arm64/include/asm/kvm_arm.h
> > index 68af5509e4b0..b8a0d410035b 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -87,6 +87,7 @@
> >  HCR_BSU_IS | HCR_FB | HCR_TACR | \
> >  HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
> >  HCR_FMO | HCR_IMO | HCR_PTW )
> > +#define HCR_GUEST_NV_FILTER_FLAGS (HCR_ATA | HCR_API | HCR_APK | HCR_RW)
> >   #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
> >   #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
> >   #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c 
> > b/arch/arm64/kvm/hyp/vhe/switch.c
> > index 57f43e607819..da80c969e623 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -36,9 +36,41 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
> > u64 hcr = vcpu->arch.hcr_el2;
> > u64 val;
> >   - /* Trap VM sysreg accesses if an EL2 guest is not using
> > VHE. */
> > -   if (vcpu_mode_el2(vcpu) && !vcpu_el2_e2h_is_set(vcpu))
> > -   hcr |= HCR_TVM | HCR_TRVM;
> > +   if (is_hyp_ctxt(vcpu)) {
> > +   hcr |= HCR_NV;
> > +
> > +   if (!vcpu_el2_e2h_is_set(vcpu)) {
> > +   /*
> > +* For a guest hypervisor on v8.0, trap and emulate
> > +* the EL1 virtual memory control register accesses.
> > +*/
> > +   hcr |= HCR_TVM | HCR_TRVM | HCR_NV1;
> > +   } else {
> > +   /*
> > +* For a guest hypervisor on v8.1 (VHE), allow to
> > +* access the EL1 virtual memory control registers
> > +* natively. These accesses are to access EL2 register
> > +* states.
> > +* Note that we still need to respect the virtual
> > +* HCR_EL2 state.
> > +*/
> > +   u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2);
> > +
> > +   vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS;
> 
> Why HCR_RW is cleared here, May I know please?

Good question. That's clearly a leftover from an early rework. It
really doesn't matter, as we are merging the guest's configuration
into the host's, and the host already has HCR_EL2.RW set.

What HCR_GUEST_NV_FILTER_FLAGS should contain is only the bits we
don't want to deal with at this stage of the NV support. I'll fix that
for the next round.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm