Re: [PATCH v6 18/18] kvm: arm64: Allow tuning the physical address size for VM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 01/11/18 08:36, Christoffer Dall wrote:
On Wed, Oct 31, 2018 at 05:55:13PM +0000, Suzuki K Poulose wrote:
Hi Christoffer,

On 31/10/18 14:22, Christoffer Dall wrote:
On Wed, Sep 26, 2018 at 05:32:54PM +0100, Suzuki K Poulose wrote:
Allow specifying the physical address size limit for a new
VM via the kvm_type argument for the KVM_CREATE_VM ioctl. This
allows us to finalise the stage2 page table as early as possible
and hence perform the right checks on the memory slots
without complication. The size is encoded as Log2(PA_Size) in
bits[7:0] of the type field. For backward compatibility the
value 0 is reserved and implies 40bits. Also, lift the limit
of the IPA to host limit and allow lower IPA sizes (e.g, 32).

The userspace could check the extension KVM_CAP_ARM_VM_IPA_SIZE
for the availability of this feature. The cap check returns the
maximum limit for the physical address shift supported by the host.

Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
Cc: Christoffer Dall <cdall@xxxxxxxxxx>
Cc: Peter Maydell <peter.maydell@xxxxxxxxxx>
Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---

@@ -192,17 +195,23 @@ int kvm_arm_config_vm(struct kvm *kvm, unsigned long type)
  	u32 parange, phys_shift;
  	u8 lvls;
-	if (type)
+	if (type & ~KVM_VM_TYPE_ARM_IPA_SIZE_MASK)
  		return -EINVAL;
+	phys_shift = KVM_VM_TYPE_ARM_IPA_SIZE(type);
+	if (phys_shift) {
+		if (phys_shift > kvm_ipa_limit ||
+		    phys_shift < 32)
+			return -EINVAL;

I am concerned here that if we allow the user to set the phys_size to 32
bits, then we end up with 2 levels of stage2 page tables, which means
that the size of a stage2 pmd mapping becomes the size of a stage2 pgd
mapping, yet we can still decide in user_mem_abort() that a stage2 fault
is backed by PMD size mappings on the host, and attempt a huge mapping
at stage2, which then becomes a PGD level block map, I think.

Yes, you're right. We will have a pgd-level block map in that case.
This should work transparently as PMD at stage2 is folded into PGD and
we endup marking the PGD entry as huge and the stage2 accessors deal
with it appropriately. This is similar to having a PMD huge page with
64K + 42bit VA (2 level page table) on stage1.


Is this handled somehow?  If so, how?

We don't prevent this. We have a guaranteed minimum number of levels
at 2, which implies you can map a stage1 PMD huge page at stage2.
I acknowledge that the Linux naming convention does cause some confusion
for a "level" at stage1 and stage2 levels. But if you think of it
from the hardware level (and like the ARM ARM defines it , Level 0-3),
it is much simpler. i.e, you can map a huge page at level N in stage1
into stage2 if you have that level N. It doesn't matter if stage2 has
more or less number of levels than stage1, as long as stage2 table can
deal with it.


That is indeed a good way to reason about it.


I can't see user_mem_abort() being modified to explicitly handle this
in your code, but perhaps the stage2_set_pmd_huge() call ends up
actually mapping at the stage2 pte level, but I can't tell that it does.

The stage2_set_pmd_huge() installs it at the PGD (level 2, which would
have been PMD if we had levels > 2) slot.

pmd = stage2_get_pmd(addr)
        \-> pud = stage2_get_pud(addr)
              \-> pgd = kvm->arch.pgd + stage2_pgd_index(addr);
              \-> (we have stage2_pgd_none(x) = 0 and
              \-> stage2_pud_offset(pgd, addr) = pgd
              \->returns (kvm->arch.pgd + stage2_pgd_index(addr);
        \->  stage_pud_none(x) = 0 & stage2_pmd_offset(pud, addr) = pud
        \->  returns pud (kvm->arch.pgd + stage2_pgd_index(addr))

and we install the PMD huge mapping at the location.

In any case, I think user_mem_abort() should give up on pmd/pud huge
mappings if the size mapped by the stage2/stage1 pmd/pud levels don't
line up.  What do you think?

Does it matter ? Personally I don't think it matters much as long as we
are able to map the "huge" page at stage1 in the stage2 as huge, even if
the stage2 has lesser levels and manage it well. Given that PMD huge
pages are quite common, it would be good to exploit it when we can.

What I couldn't convince myself of was whether having 2 levels at stage2
implied the entry level block mapping being of the same size as the

The point worth noting is, PMD (in ARM ARM terms) is always Level 2, irrespective of whether it is also PGD(the entry level). So as long as
we deal with PMD or PUD (and not PGD which varies) and make sure you
have sufficient number of levels we are fine.

For PMD it is guaranteed on all architectures and we do that explicitly
for stage2 in arm64 KVM. Similarly for PUD we need to make sure we have
3 levels to use a huge page or else fallback to splitting the pages.

stage1 block mapping, but given your explanation above, I think that's
fine.


On the other hand, for stage2 PUD we are checking if the stage2 has a
PUD level (kvm_has_stage2_pud()). May be we should relax it just like
we do for PMD to check (kvm_stage2_levels > 2).


Depends on how the code ends up looking like I suppose, but the more
symmetry we can have between the approach for host PMD and host PUD and
host PTE mappings, the better.

Sure. It is quite easy to confuse ourselves even later when we look at
it. We could put in a big fat comment explaining this to avoid
scratching our heads later.

Cheers
Suzuki
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux