2015-05-11 15:03:40

by Xiao Guangrong

[permalink] [raw]
Subject: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

KVM may turn a user page to a kernel page when kernel writes a readonly
user page if CR0.WP = 1. This shadow page entry will be reused after
SMAP is enabled so that kernel is allowed to access this user page

Fix it by setting SMAP && !CR0.WP into shadow page's role and reset mmu
once CR4.SMAP is updated

Signed-off-by: Xiao Guangrong <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/mmu.c | 16 ++++++++++++----
arch/x86/kvm/mmu.h | 2 --
arch/x86/kvm/x86.c | 8 +++-----
4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8b661d1..bbb8f4e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -207,6 +207,7 @@ union kvm_mmu_page_role {
unsigned nxe:1;
unsigned cr0_wp:1;
unsigned smep_andnot_wp:1;
+ unsigned smap_andnot_wp:1;
};
};

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 3711095..b78e83f 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3737,8 +3737,8 @@ static void reset_rsvds_bits_mask_ept(struct kvm_vcpu *vcpu,
}
}

-void update_permission_bitmask(struct kvm_vcpu *vcpu,
- struct kvm_mmu *mmu, bool ept)
+static void update_permission_bitmask(struct kvm_vcpu *vcpu,
+ struct kvm_mmu *mmu, bool ept)
{
unsigned bit, byte, pfec;
u8 map;
@@ -3919,6 +3919,7 @@ static void init_kvm_tdp_mmu(struct kvm_vcpu *vcpu)
void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu)
{
bool smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP);
+ bool smap = kvm_read_cr4_bits(vcpu, X86_CR4_SMAP);
struct kvm_mmu *context = &vcpu->arch.mmu;

MMU_WARN_ON(VALID_PAGE(context->root_hpa));
@@ -3937,6 +3938,8 @@ void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu)
context->base_role.cr0_wp = is_write_protection(vcpu);
context->base_role.smep_andnot_wp
= smep && !is_write_protection(vcpu);
+ context->base_role.smap_andnot_wp
+ = smap && !is_write_protection(vcpu);
}
EXPORT_SYMBOL_GPL(kvm_init_shadow_mmu);

@@ -4208,12 +4211,18 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
const u8 *new, int bytes)
{
gfn_t gfn = gpa >> PAGE_SHIFT;
- union kvm_mmu_page_role mask = { .word = 0 };
struct kvm_mmu_page *sp;
LIST_HEAD(invalid_list);
u64 entry, gentry, *spte;
int npte;
bool remote_flush, local_flush, zap_page;
+ union kvm_mmu_page_role mask = (union kvm_mmu_page_role) {
+ .cr0_wp = 1,
+ .cr4_pae = 1,
+ .nxe = 1,
+ .smep_andnot_wp = 1,
+ .smap_andnot_wp = 1,
+ };

/*
* If we don't have indirect shadow pages, it means no page is
@@ -4239,7 +4248,6 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
++vcpu->kvm->stat.mmu_pte_write;
kvm_mmu_audit(vcpu, AUDIT_PRE_PTE_WRITE);

- mask.cr0_wp = mask.cr4_pae = mask.nxe = mask.smep_andnot_wp = 1;
for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn) {
if (detect_write_misaligned(sp, gpa, bytes) ||
detect_write_flooding(sp)) {
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 06eb2fc..0ada65e 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -71,8 +71,6 @@ enum {
int handle_mmio_page_fault_common(struct kvm_vcpu *vcpu, u64 addr, bool direct);
void kvm_init_shadow_mmu(struct kvm_vcpu *vcpu);
void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly);
-void update_permission_bitmask(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
- bool ept);

static inline unsigned int kvm_mmu_available_pages(struct kvm *kvm)
{
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cdccbe1..cde5d61 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -702,8 +702,9 @@ EXPORT_SYMBOL_GPL(kvm_set_xcr);
int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
{
unsigned long old_cr4 = kvm_read_cr4(vcpu);
- unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE |
- X86_CR4_PAE | X86_CR4_SMEP;
+ unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
+ X86_CR4_SMEP | X86_CR4_SMAP;
+
if (cr4 & CR4_RESERVED_BITS)
return 1;

@@ -744,9 +745,6 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
(!(cr4 & X86_CR4_PCIDE) && (old_cr4 & X86_CR4_PCIDE)))
kvm_mmu_reset_context(vcpu);

- if ((cr4 ^ old_cr4) & X86_CR4_SMAP)
- update_permission_bitmask(vcpu, vcpu->arch.walk_mmu, false);
-
if ((cr4 ^ old_cr4) & X86_CR4_OSXSAVE)
kvm_update_cpuid(vcpu);

--
2.1.0


2015-05-11 15:03:20

by Xiao Guangrong

[permalink] [raw]
Subject: [PATCH 2/2] KVM: MMU: document smap_andnot_wp

Document this new role field

Signed-off-by: Xiao Guangrong <[email protected]>
---
Documentation/virtual/kvm/mmu.txt | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/mmu.txt b/Documentation/virtual/kvm/mmu.txt
index 53838d9..c59bd9b 100644
--- a/Documentation/virtual/kvm/mmu.txt
+++ b/Documentation/virtual/kvm/mmu.txt
@@ -169,6 +169,10 @@ Shadow pages contain the following information:
Contains the value of cr4.smep && !cr0.wp for which the page is valid
(pages for which this is true are different from other pages; see the
treatment of cr0.wp=0 below).
+ role.smap_andnot_wp:
+ Contains the value of cr4.smap && !cr0.wp for which the page is valid
+ (pages for which this is true are different from other pages; see the
+ treatment of cr0.wp=0 below).
gfn:
Either the guest page table containing the translations shadowed by this
page, or the base page frame for linear translations. See role.direct.
@@ -344,10 +348,16 @@ on fault type:

(user write faults generate a #PF)

-In the first case there is an additional complication if CR4.SMEP is
-enabled: since we've turned the page into a kernel page, the kernel may now
-execute it. We handle this by also setting spte.nx. If we get a user
-fetch or read fault, we'll change spte.u=1 and spte.nx=gpte.nx back.
+In the first case there are two additional complications:
+- if CR4.SMEP is enabled: since we've turned the page into a kernel page,
+ the kernel may now execute it. We handle this by also setting spte.nx.
+ If we get a user fetch or read fault, we'll change spte.u=1 and
+ spte.nx=gpte.nx back.
+- if CR4.SMAP is disabled: since the page has been changed to a kernel
+ page, it can not be reused when CR4.SMAP is enabled. We set
+ CR4.SMAP && !CR0.WP into shadow page's role to avoid this case. Note,
+ here we do not care the case that CR4.SMAP is enabled since KVM will
+ directly inject #PF to guest due to failed permission check.

To prevent an spte that was converted into a kernel page with cr0.wp=0
from being written by the kernel after cr0.wp has changed to 1, we make
--
2.1.0

2015-05-11 15:16:23

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/2] KVM: MMU: document smap_andnot_wp



On 11/05/2015 16:55, Xiao Guangrong wrote:
> Document this new role field
>
> Signed-off-by: Xiao Guangrong <[email protected]>
> ---
> Documentation/virtual/kvm/mmu.txt | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/mmu.txt b/Documentation/virtual/kvm/mmu.txt
> index 53838d9..c59bd9b 100644
> --- a/Documentation/virtual/kvm/mmu.txt
> +++ b/Documentation/virtual/kvm/mmu.txt
> @@ -169,6 +169,10 @@ Shadow pages contain the following information:
> Contains the value of cr4.smep && !cr0.wp for which the page is valid
> (pages for which this is true are different from other pages; see the
> treatment of cr0.wp=0 below).
> + role.smap_andnot_wp:
> + Contains the value of cr4.smap && !cr0.wp for which the page is valid
> + (pages for which this is true are different from other pages; see the
> + treatment of cr0.wp=0 below).
> gfn:
> Either the guest page table containing the translations shadowed by this
> page, or the base page frame for linear translations. See role.direct.
> @@ -344,10 +348,16 @@ on fault type:
>
> (user write faults generate a #PF)
>
> -In the first case there is an additional complication if CR4.SMEP is
> -enabled: since we've turned the page into a kernel page, the kernel may now
> -execute it. We handle this by also setting spte.nx. If we get a user
> -fetch or read fault, we'll change spte.u=1 and spte.nx=gpte.nx back.
> +In the first case there are two additional complications:
> +- if CR4.SMEP is enabled: since we've turned the page into a kernel page,
> + the kernel may now execute it. We handle this by also setting spte.nx.
> + If we get a user fetch or read fault, we'll change spte.u=1 and
> + spte.nx=gpte.nx back.
> +- if CR4.SMAP is disabled: since the page has been changed to a kernel
> + page, it can not be reused when CR4.SMAP is enabled. We set
> + CR4.SMAP && !CR0.WP into shadow page's role to avoid this case. Note,
> + here we do not care the case that CR4.SMAP is enabled since KVM will
> + directly inject #PF to guest due to failed permission check.
>
> To prevent an spte that was converted into a kernel page with cr0.wp=0
> from being written by the kernel after cr0.wp has changed to 1, we make
>

Thanks, squashed with 1/2 and both applied.

Paolo

2015-05-22 20:43:25

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On 05/11/2015 10:55 AM, Xiao Guangrong wrote:
> KVM may turn a user page to a kernel page when kernel writes a readonly
> user page if CR0.WP = 1. This shadow page entry will be reused after
> SMAP is enabled so that kernel is allowed to access this user page
>
> Fix it by setting SMAP && !CR0.WP into shadow page's role and reset mmu
> once CR4.SMAP is updated
>
> Signed-off-by: Xiao Guangrong <[email protected]>
> ---


>
> @@ -4208,12 +4211,18 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
> const u8 *new, int bytes)
> {
> gfn_t gfn = gpa >> PAGE_SHIFT;
> - union kvm_mmu_page_role mask = { .word = 0 };
> struct kvm_mmu_page *sp;
> LIST_HEAD(invalid_list);
> u64 entry, gentry, *spte;
> int npte;
> bool remote_flush, local_flush, zap_page;
> + union kvm_mmu_page_role mask = (union kvm_mmu_page_role) {
> + .cr0_wp = 1,
> + .cr4_pae = 1,
> + .nxe = 1,
> + .smep_andnot_wp = 1,
> + .smap_andnot_wp = 1,
> + };
>
>


This breaks older compilers that can't initialize anon structures.

-boris

2015-05-22 23:54:09

by Bandan Das

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

Boris Ostrovsky <[email protected]> writes:

> On 05/11/2015 10:55 AM, Xiao Guangrong wrote:
>> KVM may turn a user page to a kernel page when kernel writes a readonly
>> user page if CR0.WP = 1. This shadow page entry will be reused after
>> SMAP is enabled so that kernel is allowed to access this user page
>>
>> Fix it by setting SMAP && !CR0.WP into shadow page's role and reset mmu
>> once CR4.SMAP is updated
>>
>> Signed-off-by: Xiao Guangrong <[email protected]>
>> ---
>
>
>>
>> @@ -4208,12 +4211,18 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
>> const u8 *new, int bytes)
>> {
>> gfn_t gfn = gpa >> PAGE_SHIFT;
>> - union kvm_mmu_page_role mask = { .word = 0 };
>> struct kvm_mmu_page *sp;
>> LIST_HEAD(invalid_list);
>> u64 entry, gentry, *spte;
>> int npte;
>> bool remote_flush, local_flush, zap_page;
>> + union kvm_mmu_page_role mask = (union kvm_mmu_page_role) {
>> + .cr0_wp = 1,
>> + .cr4_pae = 1,
>> + .nxe = 1,
>> + .smep_andnot_wp = 1,
>> + .smap_andnot_wp = 1,
>> + };
>>
>>
>
>
> This breaks older compilers that can't initialize anon structures.

How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
3.2 is the minimum version required to compile the kernel as mentioned
in the README.

We could simply just name the structure, but I doubt this is the
only place in the kernel code where it's being used this way :)

Bandan

> -boris
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-05-23 00:42:40

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On 05/22/2015 07:54 PM, Bandan Das wrote:
> Boris Ostrovsky <[email protected]> writes:
>
>> On 05/11/2015 10:55 AM, Xiao Guangrong wrote:
>>> KVM may turn a user page to a kernel page when kernel writes a readonly
>>> user page if CR0.WP = 1. This shadow page entry will be reused after
>>> SMAP is enabled so that kernel is allowed to access this user page
>>>
>>> Fix it by setting SMAP && !CR0.WP into shadow page's role and reset mmu
>>> once CR4.SMAP is updated
>>>
>>> Signed-off-by: Xiao Guangrong <[email protected]>
>>> ---
>>
>>
>>>
>>> @@ -4208,12 +4211,18 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
>>> const u8 *new, int bytes)
>>> {
>>> gfn_t gfn = gpa >> PAGE_SHIFT;
>>> - union kvm_mmu_page_role mask = { .word = 0 };
>>> struct kvm_mmu_page *sp;
>>> LIST_HEAD(invalid_list);
>>> u64 entry, gentry, *spte;
>>> int npte;
>>> bool remote_flush, local_flush, zap_page;
>>> + union kvm_mmu_page_role mask = (union kvm_mmu_page_role) {
>>> + .cr0_wp = 1,
>>> + .cr4_pae = 1,
>>> + .nxe = 1,
>>> + .smep_andnot_wp = 1,
>>> + .smap_andnot_wp = 1,
>>> + };
>>>
>>>
>>
>>
>> This breaks older compilers that can't initialize anon structures.
>
> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
> 3.2 is the minimum version required to compile the kernel as mentioned
> in the README.
>
> We could simply just name the structure, but I doubt this is the
> only place in the kernel code where it's being used this way :)


You can use them but you can't use initializers. Unfortunately my build
system (F13) conveniently went down but this is an example from an old
email:

FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap> cat anon.c
struct bar {
struct {
int i;
};
};

main()
{
struct bar a = {.i = 0};
}

FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap> gcc --version|head -1
gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap> gcc anon.c
anon.c: In function ?main?:
anon.c:9: error: unknown field ?i? specified in initializer
FC-64 <build@build-mk2:~/xtt-x86_64/bootstrap>


but

build@build-mk2 bootstrap]$ gcc --version|head -1
gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2)
[build@build-mk2 bootstrap]$ gcc anon.c
[build@build-mk2 bootstrap]$


-boris

2015-05-26 14:45:19

by Edward Cree

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

>> This breaks older compilers that can't initialize anon structures.
>
> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
> 3.2 is the minimum version required to compile the kernel as mentioned
> in the README.
>
> We could simply just name the structure, but I doubt this is the
> only place in the kernel code where it's being used this way :)

This appears to be GCC bug #10676, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
Says it was fixed in 4.6, but I believe the kernel supports GCCs much older
than that (back to 3.2). I personally hit it on 4.4.7, the version shipped
with RHEL6.6.
So I think the kernel code has to change, probably by naming the structure.

2015-05-26 14:48:14

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization



On 26/05/2015 16:45, Edward Cree wrote:
>>> This breaks older compilers that can't initialize anon structures.
>> >
>> > How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
>> > 3.2 is the minimum version required to compile the kernel as mentioned
>> > in the README.
>> >
>> > We could simply just name the structure, but I doubt this is the
>> > only place in the kernel code where it's being used this way :)
> This appears to be GCC bug #10676, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
> Says it was fixed in 4.6, but I believe the kernel supports GCCs much older
> than that (back to 3.2). I personally hit it on 4.4.7, the version shipped
> with RHEL6.6.

Yes, it will be fixed soon(ish). Probably before you can get rid of the
obnoxious disclaimer... :)

Paolo

> The information contained in this message is confidential and is
> intended for the addressee(s) only. If you have received this message
> in error, please notify the sender immediately and delete the
> message. Unless you are an addressee (or authorized to receive for an
> addressee), you may not use, copy or disclose to anyone this message
> or any information contained in this message. The unauthorized use,
> disclosure, copying or alteration of this message is strictly
> prohibited.

2015-05-27 02:56:48

by Xiao Guangrong

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization



On 05/26/2015 10:48 PM, Paolo Bonzini wrote:
>
>
> On 26/05/2015 16:45, Edward Cree wrote:
>>>> This breaks older compilers that can't initialize anon structures.
>>>>
>>>> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
>>>> 3.2 is the minimum version required to compile the kernel as mentioned
>>>> in the README.
>>>>
>>>> We could simply just name the structure, but I doubt this is the
>>>> only place in the kernel code where it's being used this way :)
>> This appears to be GCC bug #10676, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
>> Says it was fixed in 4.6, but I believe the kernel supports GCCs much older
>> than that (back to 3.2). I personally hit it on 4.4.7, the version shipped
>> with RHEL6.6.
>
> Yes, it will be fixed soon(ish). Probably before you can get rid of the
> obnoxious disclaimer... :)

It has been fixed by Andrew:

From: Andrew Morton <[email protected]>
Subject: arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug

arch/x86/kvm/mmu.c: In function 'kvm_mmu_pte_write':
arch/x86/kvm/mmu.c:4256: error: unknown field 'cr0_wp' specified in initializer
arch/x86/kvm/mmu.c:4257: error: unknown field 'cr4_pae' specified in initializer
arch/x86/kvm/mmu.c:4257: warning: excess elements in union initializer
...

gcc-4.4.4 (at least) has issues when using anonymous unions in
initializers.

Fixes: edc90b7dc4ceef6 ("KVM: MMU: fix SMAP virtualization")
Cc: Xiao Guangrong <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

Should be found at -mm tree.

2015-06-09 05:14:57

by Vinson Lee

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On Tue, May 26, 2015 at 7:53 PM, Xiao Guangrong
<[email protected]> wrote:
>
>
> On 05/26/2015 10:48 PM, Paolo Bonzini wrote:
>>
>>
>>
>> On 26/05/2015 16:45, Edward Cree wrote:
>>>>>
>>>>> This breaks older compilers that can't initialize anon structures.
>>>>>
>>>>> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
>>>>> 3.2 is the minimum version required to compile the kernel as mentioned
>>>>> in the README.
>>>>>
>>>>> We could simply just name the structure, but I doubt this is the
>>>>> only place in the kernel code where it's being used this way :)
>>>
>>> This appears to be GCC bug #10676, see
>>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
>>> Says it was fixed in 4.6, but I believe the kernel supports GCCs much
>>> older
>>> than that (back to 3.2). I personally hit it on 4.4.7, the version
>>> shipped
>>> with RHEL6.6.
>>
>>
>> Yes, it will be fixed soon(ish). Probably before you can get rid of the
>> obnoxious disclaimer... :)
>
>
> It has been fixed by Andrew:
>
> From: Andrew Morton <[email protected]>
> Subject: arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug
>
> arch/x86/kvm/mmu.c: In function 'kvm_mmu_pte_write':
> arch/x86/kvm/mmu.c:4256: error: unknown field 'cr0_wp' specified in
> initializer
> arch/x86/kvm/mmu.c:4257: error: unknown field 'cr4_pae' specified in
> initializer
> arch/x86/kvm/mmu.c:4257: warning: excess elements in union initializer
> ...
>
> gcc-4.4.4 (at least) has issues when using anonymous unions in
> initializers.
>
> Fixes: edc90b7dc4ceef6 ("KVM: MMU: fix SMAP virtualization")
> Cc: Xiao Guangrong <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
>
> Should be found at -mm tree.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

Hi.

Please backport "arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug" to
stable trees that have backported "KVM: MMU: fix SMAP virtualization".

Currently, the Linux 4.0.5 build is broken with GCC 4.4.

"KVM: MMU: fix SMAP virtualization" is currently queued for 3.16.y-ckt
and 3.18.y.

Cheers,
Vinson

2015-06-10 18:03:06

by Davidlohr Bueso

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On Wed, 2015-05-27 at 10:53 +0800, Xiao Guangrong wrote:
>
> On 05/26/2015 10:48 PM, Paolo Bonzini wrote:
> >
> >
> > On 26/05/2015 16:45, Edward Cree wrote:
> >>>> This breaks older compilers that can't initialize anon structures.
> >>>>
> >>>> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
> >>>> 3.2 is the minimum version required to compile the kernel as mentioned
> >>>> in the README.
> >>>>
> >>>> We could simply just name the structure, but I doubt this is the
> >>>> only place in the kernel code where it's being used this way :)
> >> This appears to be GCC bug #10676, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
> >> Says it was fixed in 4.6, but I believe the kernel supports GCCs much older
> >> than that (back to 3.2). I personally hit it on 4.4.7, the version shipped
> >> with RHEL6.6.
> >
> > Yes, it will be fixed soon(ish). Probably before you can get rid of the
> > obnoxious disclaimer... :)
>
> It has been fixed by Andrew:
>
> From: Andrew Morton <[email protected]>
> Subject: arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug

Folks, this fix is missing in both -tip and Linus' (4.1-rc7) tree. At
least I'm running into it.

Thanks,
Davidlohr

2015-06-10 18:08:59

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On Wed, 10 Jun 2015 11:02:46 -0700 Davidlohr Bueso <[email protected]> wrote:

> On Wed, 2015-05-27 at 10:53 +0800, Xiao Guangrong wrote:
> >
> > On 05/26/2015 10:48 PM, Paolo Bonzini wrote:
> > >
> > >
> > > On 26/05/2015 16:45, Edward Cree wrote:
> > >>>> This breaks older compilers that can't initialize anon structures.
> > >>>>
> > >>>> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
> > >>>> 3.2 is the minimum version required to compile the kernel as mentioned
> > >>>> in the README.
> > >>>>
> > >>>> We could simply just name the structure, but I doubt this is the
> > >>>> only place in the kernel code where it's being used this way :)
> > >> This appears to be GCC bug #10676, see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
> > >> Says it was fixed in 4.6, but I believe the kernel supports GCCs much older
> > >> than that (back to 3.2). I personally hit it on 4.4.7, the version shipped
> > >> with RHEL6.6.
> > >
> > > Yes, it will be fixed soon(ish). Probably before you can get rid of the
> > > obnoxious disclaimer... :)
> >
> > It has been fixed by Andrew:
> >
> > From: Andrew Morton <[email protected]>
> > Subject: arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug
>
> Folks, this fix is missing in both -tip and Linus' (4.1-rc7) tree. At
> least I'm running into it.

hm, yes, it's in linux-next so I dropped my copy.

I'll send it in to Linus today.

2015-06-12 12:11:17

by Luis Henriques

[permalink] [raw]
Subject: Re: [PATCH 1/2] KVM: MMU: fix SMAP virtualization

On Mon, Jun 08, 2015 at 10:14:49PM -0700, Vinson Lee wrote:
> On Tue, May 26, 2015 at 7:53 PM, Xiao Guangrong
> <[email protected]> wrote:
> >
> >
> > On 05/26/2015 10:48 PM, Paolo Bonzini wrote:
> >>
> >>
> >>
> >> On 26/05/2015 16:45, Edward Cree wrote:
> >>>>>
> >>>>> This breaks older compilers that can't initialize anon structures.
> >>>>>
> >>>>> How old ? Even gcc 3.1 says you can use unnamed struct/union fields and
> >>>>> 3.2 is the minimum version required to compile the kernel as mentioned
> >>>>> in the README.
> >>>>>
> >>>>> We could simply just name the structure, but I doubt this is the
> >>>>> only place in the kernel code where it's being used this way :)
> >>>
> >>> This appears to be GCC bug #10676, see
> >>> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676>
> >>> Says it was fixed in 4.6, but I believe the kernel supports GCCs much
> >>> older
> >>> than that (back to 3.2). I personally hit it on 4.4.7, the version
> >>> shipped
> >>> with RHEL6.6.
> >>
> >>
> >> Yes, it will be fixed soon(ish). Probably before you can get rid of the
> >> obnoxious disclaimer... :)
> >
> >
> > It has been fixed by Andrew:
> >
> > From: Andrew Morton <[email protected]>
> > Subject: arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug
> >
> > arch/x86/kvm/mmu.c: In function 'kvm_mmu_pte_write':
> > arch/x86/kvm/mmu.c:4256: error: unknown field 'cr0_wp' specified in
> > initializer
> > arch/x86/kvm/mmu.c:4257: error: unknown field 'cr4_pae' specified in
> > initializer
> > arch/x86/kvm/mmu.c:4257: warning: excess elements in union initializer
> > ...
> >
> > gcc-4.4.4 (at least) has issues when using anonymous unions in
> > initializers.
> >
> > Fixes: edc90b7dc4ceef6 ("KVM: MMU: fix SMAP virtualization")
> > Cc: Xiao Guangrong <[email protected]>
> > Cc: Paolo Bonzini <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> >
> > Should be found at -mm tree.
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> Hi.
>
> Please backport "arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug" to
> stable trees that have backported "KVM: MMU: fix SMAP virtualization".
>
> Currently, the Linux 4.0.5 build is broken with GCC 4.4.
>
> "KVM: MMU: fix SMAP virtualization" is currently queued for 3.16.y-ckt
> and 3.18.y.
>
> Cheers,
> Vinson
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

I believe you're referring to the following commit:

commit 5ec45a192fe6e287f0fc06d5ca4f3bd446d94803
Author: Andrew Morton <[email protected]>
Date: Wed Jun 10 11:15:02 2015 -0700

arch/x86/kvm/mmu.c: work around gcc-4.4.4 bug

I am queuing it for the 3.16 kernel. Thanks!

Cheers,
--
Lu?s