diff mbox

[4.2.y-ckt,stable] Patch "ARM/arm64: KVM: test properly for a PTE's uncachedness" has been added to staging queue

Message ID 1452813303-10936-1-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa Jan. 14, 2016, 11:15 p.m. UTC 
This is a note to let you know that I have just added a patch titled

    ARM/arm64: KVM: test properly for a PTE's uncachedness

to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree 
which can be found at:

    http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue

This patch is scheduled to be released in version 4.2.8-ckt2.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 4.2.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

---8<------------------------------------------------------------

From 6cbe0242b8048e540987f9da422f067af5cda6cb Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Tue, 10 Nov 2015 15:11:20 +0100
Subject: ARM/arm64: KVM: test properly for a PTE's uncachedness

commit e6fab54423450d699a09ec2b899473a541f61971 upstream.

The open coded tests for checking whether a PTE maps a page as
uncached use a flawed '(pte_val(xxx) & CONST) != CONST' pattern,
which is not guaranteed to work since the type of a mapping is
not a set of mutually exclusive bits

For HYP mappings, the type is an index into the MAIR table (i.e, the
index itself does not contain any information whatsoever about the
type of the mapping), and for stage-2 mappings it is a bit field where
normal memory and device types are defined as follows:

    #define MT_S2_NORMAL            0xf
    #define MT_S2_DEVICE_nGnRE      0x1

I.e., masking *and* comparing with the latter matches on the former,
and we have been getting lucky merely because the S2 device mappings
also have the PTE_UXN bit set, or we would misidentify memory mappings
as device mappings.

Since the unmap_range() code path (which contains one instance of the
flawed test) is used both for HYP mappings and stage-2 mappings, and
considering the difference between the two, it is non-trivial to fix
this by rewriting the tests in place, as it would involve passing
down the type of mapping through all the functions.

However, since HYP mappings and stage-2 mappings both deal with host
physical addresses, we can simply check whether the mapping is backed
by memory that is managed by the host kernel, and only perform the
D-cache maintenance if this is the case.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Pavel Fedin <p.fedin@samsung.com>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 arch/arm/kvm/mmu.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

--
1.9.1

Comments

Christoffer Dall Jan. 15, 2016, 11:27 a.m. UTC | #1 
On Thu, Jan 14, 2016 at 03:15:03PM -0800, Kamal Mostafa wrote:
> This is a note to let you know that I have just added a patch titled
> 
>     ARM/arm64: KVM: test properly for a PTE's uncachedness
> 
> to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree 
> which can be found at:
> 
>     http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue
> 
> This patch is scheduled to be released in version 4.2.8-ckt2.
> 
> If you, or anyone else, feels it should not be added to this tree, please 
> reply to this email.

This fix was incorrect and was subsequently updated by the following
commit in mainline:

0de58f8 (ARM/arm64: KVM: correct PTE uncachedness check, 2015-12-03)

That update should be applied along with this one.

Thanks,
-Christoffer

> 
> ---8<------------------------------------------------------------
> 
> From 6cbe0242b8048e540987f9da422f067af5cda6cb Mon Sep 17 00:00:00 2001
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Tue, 10 Nov 2015 15:11:20 +0100
> Subject: ARM/arm64: KVM: test properly for a PTE's uncachedness
> 
> commit e6fab54423450d699a09ec2b899473a541f61971 upstream.
> 
> The open coded tests for checking whether a PTE maps a page as
> uncached use a flawed '(pte_val(xxx) & CONST) != CONST' pattern,
> which is not guaranteed to work since the type of a mapping is
> not a set of mutually exclusive bits
> 
> For HYP mappings, the type is an index into the MAIR table (i.e, the
> index itself does not contain any information whatsoever about the
> type of the mapping), and for stage-2 mappings it is a bit field where
> normal memory and device types are defined as follows:
> 
>     #define MT_S2_NORMAL            0xf
>     #define MT_S2_DEVICE_nGnRE      0x1
> 
> I.e., masking *and* comparing with the latter matches on the former,
> and we have been getting lucky merely because the S2 device mappings
> also have the PTE_UXN bit set, or we would misidentify memory mappings
> as device mappings.
> 
> Since the unmap_range() code path (which contains one instance of the
> flawed test) is used both for HYP mappings and stage-2 mappings, and
> considering the difference between the two, it is non-trivial to fix
> this by rewriting the tests in place, as it would involve passing
> down the type of mapping through all the functions.
> 
> However, since HYP mappings and stage-2 mappings both deal with host
> physical addresses, we can simply check whether the mapping is backed
> by memory that is managed by the host kernel, and only perform the
> D-cache maintenance if this is the case.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Tested-by: Pavel Fedin <p.fedin@samsung.com>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  arch/arm/kvm/mmu.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index 6984342..7dace90 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -98,6 +98,11 @@ static void kvm_flush_dcache_pud(pud_t pud)
>  	__kvm_flush_dcache_pud(pud);
>  }
> 
> +static bool kvm_is_device_pfn(unsigned long pfn)
> +{
> +	return !pfn_valid(pfn);
> +}
> +
>  /**
>   * stage2_dissolve_pmd() - clear and flush huge PMD entry
>   * @kvm:	pointer to kvm structure.
> @@ -213,7 +218,7 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd,
>  			kvm_tlb_flush_vmid_ipa(kvm, addr);
> 
>  			/* No need to invalidate the cache for device mappings */
> -			if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> +			if (!kvm_is_device_pfn(__phys_to_pfn(addr)))
>  				kvm_flush_dcache_pte(old_pte);
> 
>  			put_page(virt_to_page(pte));
> @@ -305,8 +310,7 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,
> 
>  	pte = pte_offset_kernel(pmd, addr);
>  	do {
> -		if (!pte_none(*pte) &&
> -		    (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> +		if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr)))
>  			kvm_flush_dcache_pte(*pte);
>  	} while (pte++, addr += PAGE_SIZE, addr != end);
>  }
> @@ -1037,11 +1041,6 @@ 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);
> -}
> -
>  /**
>   * stage2_wp_ptes - write protect PMD range
>   * @pmd:	pointer to pmd entry
> --
> 1.9.1
>
Kamal Mostafa Jan. 15, 2016, 5:19 p.m. UTC | #2 
On Fri, 2016-01-15 at 12:27 +0100, Christoffer Dall wrote:
> On Thu, Jan 14, 2016 at 03:15:03PM -0800, Kamal Mostafa wrote:
> > This is a note to let you know that I have just added a patch titled
> > 
> >     ARM/arm64: KVM: test properly for a PTE's uncachedness
> > 
> > to the linux-4.2.y-queue branch of the 4.2.y-ckt extended stable tree 
> > which can be found at:
> > 
> >     http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-4.2.y-queue
> > 
> > This patch is scheduled to be released in version 4.2.8-ckt2.
> > 
> > If you, or anyone else, feels it should not be added to this tree, please 
> > reply to this email.
> 
> This fix was incorrect and was subsequently updated by the following
> commit in mainline:
> 
> 0de58f8 (ARM/arm64: KVM: correct PTE uncachedness check, 2015-12-03)
> 
> That update should be applied along with this one.
> 
> Thanks,
> -Christoffer

Ok, I'll pick up "correct PTE uncachedness" too.  Thanks Christoffer!

 -Kamal


> > 
> > ---8<------------------------------------------------------------
> > 
> > From 6cbe0242b8048e540987f9da422f067af5cda6cb Mon Sep 17 00:00:00 2001
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Date: Tue, 10 Nov 2015 15:11:20 +0100
> > Subject: ARM/arm64: KVM: test properly for a PTE's uncachedness
> > 
> > commit e6fab54423450d699a09ec2b899473a541f61971 upstream.
> > 
> > The open coded tests for checking whether a PTE maps a page as
> > uncached use a flawed '(pte_val(xxx) & CONST) != CONST' pattern,
> > which is not guaranteed to work since the type of a mapping is
> > not a set of mutually exclusive bits
> > 
> > For HYP mappings, the type is an index into the MAIR table (i.e, the
> > index itself does not contain any information whatsoever about the
> > type of the mapping), and for stage-2 mappings it is a bit field where
> > normal memory and device types are defined as follows:
> > 
> >     #define MT_S2_NORMAL            0xf
> >     #define MT_S2_DEVICE_nGnRE      0x1
> > 
> > I.e., masking *and* comparing with the latter matches on the former,
> > and we have been getting lucky merely because the S2 device mappings
> > also have the PTE_UXN bit set, or we would misidentify memory mappings
> > as device mappings.
> > 
> > Since the unmap_range() code path (which contains one instance of the
> > flawed test) is used both for HYP mappings and stage-2 mappings, and
> > considering the difference between the two, it is non-trivial to fix
> > this by rewriting the tests in place, as it would involve passing
> > down the type of mapping through all the functions.
> > 
> > However, since HYP mappings and stage-2 mappings both deal with host
> > physical addresses, we can simply check whether the mapping is backed
> > by memory that is managed by the host kernel, and only perform the
> > D-cache maintenance if this is the case.
> > 
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Tested-by: Pavel Fedin <p.fedin@samsung.com>
> > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> > ---
> >  arch/arm/kvm/mmu.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> > index 6984342..7dace90 100644
> > --- a/arch/arm/kvm/mmu.c
> > +++ b/arch/arm/kvm/mmu.c
> > @@ -98,6 +98,11 @@ static void kvm_flush_dcache_pud(pud_t pud)
> >  	__kvm_flush_dcache_pud(pud);
> >  }
> > 
> > +static bool kvm_is_device_pfn(unsigned long pfn)
> > +{
> > +	return !pfn_valid(pfn);
> > +}
> > +
> >  /**
> >   * stage2_dissolve_pmd() - clear and flush huge PMD entry
> >   * @kvm:	pointer to kvm structure.
> > @@ -213,7 +218,7 @@ static void unmap_ptes(struct kvm *kvm, pmd_t *pmd,
> >  			kvm_tlb_flush_vmid_ipa(kvm, addr);
> > 
> >  			/* No need to invalidate the cache for device mappings */
> > -			if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> > +			if (!kvm_is_device_pfn(__phys_to_pfn(addr)))
> >  				kvm_flush_dcache_pte(old_pte);
> > 
> >  			put_page(virt_to_page(pte));
> > @@ -305,8 +310,7 @@ static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,
> > 
> >  	pte = pte_offset_kernel(pmd, addr);
> >  	do {
> > -		if (!pte_none(*pte) &&
> > -		    (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
> > +		if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr)))
> >  			kvm_flush_dcache_pte(*pte);
> >  	} while (pte++, addr += PAGE_SIZE, addr != end);
> >  }
> > @@ -1037,11 +1041,6 @@ 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);
> > -}
> > -
> >  /**
> >   * stage2_wp_ptes - write protect PMD range
> >   * @pmd:	pointer to pmd entry
> > --
> > 1.9.1
> > 
>
diff mbox

Patch

diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index 6984342..7dace90 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -98,6 +98,11 @@  static void kvm_flush_dcache_pud(pud_t pud)
 	__kvm_flush_dcache_pud(pud);
 }

+static bool kvm_is_device_pfn(unsigned long pfn)
+{
+	return !pfn_valid(pfn);
+}
+
 /**
  * stage2_dissolve_pmd() - clear and flush huge PMD entry
  * @kvm:	pointer to kvm structure.
@@ -213,7 +218,7 @@  static void unmap_ptes(struct kvm *kvm, pmd_t *pmd,
 			kvm_tlb_flush_vmid_ipa(kvm, addr);

 			/* No need to invalidate the cache for device mappings */
-			if ((pte_val(old_pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
+			if (!kvm_is_device_pfn(__phys_to_pfn(addr)))
 				kvm_flush_dcache_pte(old_pte);

 			put_page(virt_to_page(pte));
@@ -305,8 +310,7 @@  static void stage2_flush_ptes(struct kvm *kvm, pmd_t *pmd,

 	pte = pte_offset_kernel(pmd, addr);
 	do {
-		if (!pte_none(*pte) &&
-		    (pte_val(*pte) & PAGE_S2_DEVICE) != PAGE_S2_DEVICE)
+		if (!pte_none(*pte) && !kvm_is_device_pfn(__phys_to_pfn(addr)))
 			kvm_flush_dcache_pte(*pte);
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 }
@@ -1037,11 +1041,6 @@  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);
-}
-
 /**
  * stage2_wp_ptes - write protect PMD range
  * @pmd:	pointer to pmd entry