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

Andrew Scull
2020-05-05T15:26:58Z

Having a go at reviewing. Might turn out to be more useful as a learning
exercise for me rather than useful feedback but we've got to start
somewhere..

> -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.
> + */
> + pgd_t *pgd;
> + phys_addr_t pgd_phys;
>
> /* The last vcpu id that ran on each physical CPU */
> int __percpu *last_vcpu_ran;
>
> + struct kvm *kvm;
> +};
> +
> +struct kvm_arch {
> + struct kvm_s2_mmu mmu;
> +
> + /* VTCR_EL2 value for this VM */
> + u64 vtcr;

VTCR seems quite strongly tied to the MMU config. Is it not controlled
independently for the nested MMUs and so remains in this struct?

> -static void stage2_dissolve_pmd(struct kvm *kvm, phys_addr_t addr, pmd_t *pmd)
> +static void stage2_dissolve_pmd(struct kvm_s2_mmu *mmu, phys_addr_t addr, pmd_t *pmd)

How strictly is the long line style rule enforced? checkpatch has 16
such warnings on this patch.

> -static void stage2_dissolve_pud(struct kvm *kvm, phys_addr_t addr, pud_t *pudp)
> +static void stage2_dissolve_pud(struct kvm_s2_mmu *mmu, phys_addr_t addr, pud_t *pudp)
> {
> + struct kvm *kvm __maybe_unused = mmu->kvm;
> +
> if (!stage2_pud_huge(kvm, *pudp))
> return;

There're a couple places with `__maybe_unused` on variables that are
then used soon after. Can they be dropped in these cases so as not to
hide legitimate warning?