Re: [PATCH 03/26] KVM: arm64: Factor out stage 2 page table data from struct kvm

Alexandru Elisei
2020-05-12T16:53:28Z

Hi,

On 5/12/20 12:17 PM, James Morse wrote:
> Hi Alex, Marc,
>
> (just on this last_vcpu_ran thing...)
>
> On 11/05/2020 17:38, Alexandru Elisei wrote:
>> On 4/22/20 1:00 PM, Marc Zyngier wrote:
>>> From: Christoffer Dall <christoffer.dall@arm.com>
>>>
>>> As we are about to reuse our stage 2 page table manipulation code for
>>> shadow stage 2 page tables in the context of nested virtualization, we
>>> are going to manage multiple stage 2 page tables for a single VM.
>>>
>>> This requires some pretty invasive changes to our data structures,
>>> which moves the vmid and pgd pointers into a separate structure and
>>> change pretty much all of our mmu code to operate on this structure
>>> instead.
>>>
>>> The new structure is called struct kvm_s2_mmu.
>>>
>>> There is no intended functional change by this patch alone.
>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>> index 7dd8fefa6aecd..664a5d92ae9b8 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -63,19 +63,32 @@ struct kvm_vmid {
>>> u32 vmid;
>>> };
>>>
>>> -struct kvm_arch {
>>> +struct kvm_s2_mmu {
>>> struct kvm_vmid vmid;
>>>
>>> - /* stage2 entry level table */
>>> - pgd_t *pgd;
>>> - phys_addr_t pgd_phys;
>>> -
>>> - /* VTCR_EL2 value for this VM */
>>> - u64 vtcr;
>>> + /*
>>> + * stage2 entry level table
>>> + *
>>> + * Two kvm_s2_mmu structures in the same VM can point to the same pgd
>>> + * here. This happens when running a non-VHE guest hypervisor which
>>> + * uses the canonical stage 2 page table for both vEL2 and for vEL1/0
>>> + * with vHCR_EL2.VM == 0.
>> It makes more sense to me to say that a non-VHE guest hypervisor will use the
>> canonical stage *1* page table when running at EL2
> Can KVM say anything about stage1? Its totally under the the guests control even at vEL2...

It just occurred to me that "canonical stage 2 page table" refers to the L0
hypervisor stage 2, not to the L1 hypervisor stage 2. If you don't mind my
suggestion, perhaps the comment can be slightly improved to avoid any confusion?
Maybe something along the lines of "[..] This happens when running a non-VHE guest
hypervisor, in which case we use the canonical stage 2 page table for both vEL2
and for vEL1/0 with vHCR_EL2.VM == 0".

Thanks,
Alex