| Message ID | 1415608436-5127-1-git-send-email-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: > Instead of using kvm_is_mmio_pfn() to decide whether a host region > should be stage 2 mapped with device attributes, add a new static > function kvm_is_device_pfn() that disregards RAM pages with the > reserved bit set, as those should usually not be mapped as device > memory. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm/kvm/mmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 57a403a5c22b..b007438242e2 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) > return kvm_vcpu_dabt_iswrite(vcpu); > } > > +static bool kvm_is_device_pfn(unsigned long pfn) > +{ > + return !pfn_valid(pfn); > +} So this works for Magnus' use case, because a device tree memreserve results in reserved, but valid, existing pages being backed by a struct page? > + > static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > struct kvm_memory_slot *memslot, unsigned long hva, > unsigned long fault_status) > @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > if (is_error_pfn(pfn)) > return -EFAULT; > > - if (kvm_is_mmio_pfn(pfn)) > + if (kvm_is_device_pfn(pfn)) > mem_type = PAGE_S2_DEVICE; > > spin_lock(&kvm->mmu_lock); > -- > 1.8.3.2 > If my understanding above is correct, then: Acked-by: Christoffer Dall <christoffer.dall@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10 November 2014 11:57, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: >> Instead of using kvm_is_mmio_pfn() to decide whether a host region >> should be stage 2 mapped with device attributes, add a new static >> function kvm_is_device_pfn() that disregards RAM pages with the >> reserved bit set, as those should usually not be mapped as device >> memory. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> arch/arm/kvm/mmu.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index 57a403a5c22b..b007438242e2 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) >> return kvm_vcpu_dabt_iswrite(vcpu); >> } >> >> +static bool kvm_is_device_pfn(unsigned long pfn) >> +{ >> + return !pfn_valid(pfn); >> +} > > So this works for Magnus' use case, because a device tree memreserve > results in reserved, but valid, existing pages being backed by a struct > page? > That is the idea, yes, but it would be good if he could confirm that it works as expected. Also, there may be some corner cases where pfn_valid returns false for regions that are in fact mapped as MT_NORMAL by the host kernel, i.e., UEFI configuration tables, for instance, so this test may require further refinement. But it at least eliminates the false positives for plain memreserve regions. >> + >> static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> struct kvm_memory_slot *memslot, unsigned long hva, >> unsigned long fault_status) >> @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> if (is_error_pfn(pfn)) >> return -EFAULT; >> >> - if (kvm_is_mmio_pfn(pfn)) >> + if (kvm_is_device_pfn(pfn)) >> mem_type = PAGE_S2_DEVICE; >> >> spin_lock(&kvm->mmu_lock); >> -- >> 1.8.3.2 >> > > If my understanding above is correct, then: > > Acked-by: Christoffer Dall <christoffer.dall@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: > Instead of using kvm_is_mmio_pfn() to decide whether a host region > should be stage 2 mapped with device attributes, add a new static > function kvm_is_device_pfn() that disregards RAM pages with the > reserved bit set, as those should usually not be mapped as device > memory. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm/kvm/mmu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index 57a403a5c22b..b007438242e2 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) > return kvm_vcpu_dabt_iswrite(vcpu); > } > > +static bool kvm_is_device_pfn(unsigned long pfn) > +{ > + return !pfn_valid(pfn); > +} > + > static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > struct kvm_memory_slot *memslot, unsigned long hva, > unsigned long fault_status) > @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > if (is_error_pfn(pfn)) > return -EFAULT; > > - if (kvm_is_mmio_pfn(pfn)) > + if (kvm_is_device_pfn(pfn)) > mem_type = PAGE_S2_DEVICE; > > spin_lock(&kvm->mmu_lock); > -- > 1.8.3.2 > Acked-by: Christoffer Dall <christoffer.dall@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21 November 2014 at 12:24, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: >> Instead of using kvm_is_mmio_pfn() to decide whether a host region >> should be stage 2 mapped with device attributes, add a new static >> function kvm_is_device_pfn() that disregards RAM pages with the >> reserved bit set, as those should usually not be mapped as device >> memory. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> arch/arm/kvm/mmu.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index 57a403a5c22b..b007438242e2 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) >> return kvm_vcpu_dabt_iswrite(vcpu); >> } >> >> +static bool kvm_is_device_pfn(unsigned long pfn) >> +{ >> + return !pfn_valid(pfn); >> +} >> + >> static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> struct kvm_memory_slot *memslot, unsigned long hva, >> unsigned long fault_status) >> @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> if (is_error_pfn(pfn)) >> return -EFAULT; >> >> - if (kvm_is_mmio_pfn(pfn)) >> + if (kvm_is_device_pfn(pfn)) >> mem_type = PAGE_S2_DEVICE; >> >> spin_lock(&kvm->mmu_lock); >> -- >> 1.8.3.2 >> > Acked-by: Christoffer Dall <christoffer.dall@linaro.org> These 2 patches are now in 3.18-rc7, so they can be dropped from the kvmarm queue/next/etc branches Thanks, Ard. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/12/2014 10:16, Ard Biesheuvel wrote: > On 21 November 2014 at 12:24, Christoffer Dall > <christoffer.dall@linaro.org> wrote: >> On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: >>> Instead of using kvm_is_mmio_pfn() to decide whether a host region >>> should be stage 2 mapped with device attributes, add a new static >>> function kvm_is_device_pfn() that disregards RAM pages with the >>> reserved bit set, as those should usually not be mapped as device >>> memory. >>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> --- >>> arch/arm/kvm/mmu.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >>> index 57a403a5c22b..b007438242e2 100644 >>> --- a/arch/arm/kvm/mmu.c >>> +++ b/arch/arm/kvm/mmu.c >>> @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) >>> return kvm_vcpu_dabt_iswrite(vcpu); >>> } >>> >>> +static bool kvm_is_device_pfn(unsigned long pfn) >>> +{ >>> + return !pfn_valid(pfn); >>> +} >>> + >>> static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >>> struct kvm_memory_slot *memslot, unsigned long hva, >>> unsigned long fault_status) >>> @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >>> if (is_error_pfn(pfn)) >>> return -EFAULT; >>> >>> - if (kvm_is_mmio_pfn(pfn)) >>> + if (kvm_is_device_pfn(pfn)) >>> mem_type = PAGE_S2_DEVICE; >>> >>> spin_lock(&kvm->mmu_lock); >>> -- >>> 1.8.3.2 >>> >> Acked-by: Christoffer Dall <christoffer.dall@linaro.org> > > These 2 patches are now in 3.18-rc7, so they can be dropped from the > kvmarm queue/next/etc branches If they are in queue, they can be dropped. If they are in next, please leave them in as the next branch should not be rebased. Duplicate commits are generally harmless. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 01, 2014 at 10:54:44AM +0100, Paolo Bonzini wrote: > > > On 01/12/2014 10:16, Ard Biesheuvel wrote: > > On 21 November 2014 at 12:24, Christoffer Dall > > <christoffer.dall@linaro.org> wrote: > >> On Mon, Nov 10, 2014 at 09:33:55AM +0100, Ard Biesheuvel wrote: > >>> Instead of using kvm_is_mmio_pfn() to decide whether a host region > >>> should be stage 2 mapped with device attributes, add a new static > >>> function kvm_is_device_pfn() that disregards RAM pages with the > >>> reserved bit set, as those should usually not be mapped as device > >>> memory. > >>> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >>> --- > >>> arch/arm/kvm/mmu.c | 7 ++++++- > >>> 1 file changed, 6 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >>> index 57a403a5c22b..b007438242e2 100644 > >>> --- a/arch/arm/kvm/mmu.c > >>> +++ b/arch/arm/kvm/mmu.c > >>> @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) > >>> return kvm_vcpu_dabt_iswrite(vcpu); > >>> } > >>> > >>> +static bool kvm_is_device_pfn(unsigned long pfn) > >>> +{ > >>> + return !pfn_valid(pfn); > >>> +} > >>> + > >>> static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > >>> struct kvm_memory_slot *memslot, unsigned long hva, > >>> unsigned long fault_status) > >>> @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > >>> if (is_error_pfn(pfn)) > >>> return -EFAULT; > >>> > >>> - if (kvm_is_mmio_pfn(pfn)) > >>> + if (kvm_is_device_pfn(pfn)) > >>> mem_type = PAGE_S2_DEVICE; > >>> > >>> spin_lock(&kvm->mmu_lock); > >>> -- > >>> 1.8.3.2 > >>> > >> Acked-by: Christoffer Dall <christoffer.dall@linaro.org> > > > > These 2 patches are now in 3.18-rc7, so they can be dropped from the > > kvmarm queue/next/etc branches > > If they are in queue, they can be dropped. If they are in next, please > leave them in as the next branch should not be rebased. Duplicate > commits are generally harmless. > They are in kvmarm/next and will stay there ;) -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index 57a403a5c22b..b007438242e2 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -834,6 +834,11 @@ static bool kvm_is_write_fault(struct kvm_vcpu *vcpu) return kvm_vcpu_dabt_iswrite(vcpu); } +static bool kvm_is_device_pfn(unsigned long pfn) +{ + return !pfn_valid(pfn); +} + static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, struct kvm_memory_slot *memslot, unsigned long hva, unsigned long fault_status) @@ -904,7 +909,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (is_error_pfn(pfn)) return -EFAULT; - if (kvm_is_mmio_pfn(pfn)) + if (kvm_is_device_pfn(pfn)) mem_type = PAGE_S2_DEVICE; spin_lock(&kvm->mmu_lock);
Instead of using kvm_is_mmio_pfn() to decide whether a host region should be stage 2 mapped with device attributes, add a new static function kvm_is_device_pfn() that disregards RAM pages with the reserved bit set, as those should usually not be mapped as device memory. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm/kvm/mmu.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)