| Message ID | 20220714053631.417152-1-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/i386: Restore TSX features with taa-no | expand |
On 7/14/22 07:36, Zhenzhong Duan wrote: > On ICX-2S2 host, when run L2 guest with both L1/L2 using Icelake-Server-v3 > or above, we got below warning: > > "warning: host doesn't support requested feature: MSR(10AH).taa-no [bit 8]" > > This is because L1 KVM doesn't expose taa-no to L2 if RTM is disabled, > then starting L2 qemu triggers the warning. > > Fix it by restoring TSX features in Icelake-Server-v3, which may also help > guest performance if host isn't susceptible to TSX Async Abort (TAA) > vulnerabilities. > > Fixes: d965dc35592d ("target/i386: Add ARCH_CAPABILITIES related bits into Icelake-Server CPU model") > Tested-by: Xiangfei Ma <xiangfeix.ma@intel.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > v2: Rewrite commit message Why wouldn't the fix be (in an Icelake-Server-v4 model) to remove taa-no? Paolo > target/i386/cpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 14f681e998cc..25ef972a3eed 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -3423,6 +3423,9 @@ static const X86CPUDefinition builtin_x86_defs[] = { > { > .version = 3, > .props = (PropValue[]) { > + /* Restore TSX features removed by -v2 above */ > + { "hle", "on" }, > + { "rtm", "on" }, > { "arch-capabilities", "on" }, > { "rdctl-no", "on" }, > { "ibrs-all", "on" },
>-----Original Message----- >From: Paolo Bonzini <paolo.bonzini@gmail.com> On Behalf Of Paolo Bonzini >Sent: Thursday, July 14, 2022 3:59 PM >To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; qemu- >devel@nongnu.org >Cc: ehabkost@redhat.com; Ma, XiangfeiX <xiangfeix.ma@intel.com>; Li, >Xiaoyao <xiaoyao.li@intel.com>; Christopherson,, Sean <seanjc@google.com> >Subject: Re: [PATCH v2] target/i386: Restore TSX features with taa-no > >On 7/14/22 07:36, Zhenzhong Duan wrote: >> On ICX-2S2 host, when run L2 guest with both L1/L2 using >> Icelake-Server-v3 or above, we got below warning: >> >> "warning: host doesn't support requested feature: MSR(10AH).taa-no [bit >8]" >> >> This is because L1 KVM doesn't expose taa-no to L2 if RTM is disabled, >> then starting L2 qemu triggers the warning. >> >> Fix it by restoring TSX features in Icelake-Server-v3, which may also >> help guest performance if host isn't susceptible to TSX Async Abort >> (TAA) vulnerabilities. >> >> Fixes: d965dc35592d ("target/i386: Add ARCH_CAPABILITIES related bits >> into Icelake-Server CPU model") >> Tested-by: Xiangfei Ma <xiangfeix.ma@intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> v2: Rewrite commit message > >Why wouldn't the fix be (in an Icelake-Server-v4 model) to remove taa-no? This way we don't have a versioned model enabling both TSX and taa-no. In currently implementation, TSX is disabled in Icelake-Server-v2 and above. And taa-no is enabled in Icelake-Server-v3 and above. If hardware supports taa-no mitigation, I thought it's better to expose it to guest together with TSX so that guest knows it's secure to use TSX? Thanks Zhenzhong > >Paolo > >> target/i386/cpu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c index >> 14f681e998cc..25ef972a3eed 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -3423,6 +3423,9 @@ static const X86CPUDefinition builtin_x86_defs[] >= { >> { >> .version = 3, >> .props = (PropValue[]) { >> + /* Restore TSX features removed by -v2 above */ >> + { "hle", "on" }, >> + { "rtm", "on" }, >> { "arch-capabilities", "on" }, >> { "rdctl-no", "on" }, >> { "ibrs-all", "on" },
On 7/14/2022 3:59 PM, Paolo Bonzini wrote: > On 7/14/22 07:36, Zhenzhong Duan wrote: >> On ICX-2S2 host, when run L2 guest with both L1/L2 using >> Icelake-Server-v3 >> or above, we got below warning: >> >> "warning: host doesn't support requested feature: MSR(10AH).taa-no >> [bit 8]" >> >> This is because L1 KVM doesn't expose taa-no to L2 if RTM is disabled, >> then starting L2 qemu triggers the warning. >> >> Fix it by restoring TSX features in Icelake-Server-v3, which may also >> help >> guest performance if host isn't susceptible to TSX Async Abort (TAA) >> vulnerabilities. >> >> Fixes: d965dc35592d ("target/i386: Add ARCH_CAPABILITIES related bits >> into Icelake-Server CPU model") >> Tested-by: Xiangfei Ma <xiangfeix.ma@intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> v2: Rewrite commit message > > Why wouldn't the fix be (in an Icelake-Server-v4 model) to remove taa-no? Production Icelake silicon should have the taa-no set, that's the reason taa-no was added in v3 model. When taa-no presents, it's safe to bring TSX features back. I'm wondering if we need a new version (v7) for this change. > Paolo > >> target/i386/cpu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 14f681e998cc..25ef972a3eed 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -3423,6 +3423,9 @@ static const X86CPUDefinition builtin_x86_defs[] >> = { >> { >> .version = 3, >> .props = (PropValue[]) { >> + /* Restore TSX features removed by -v2 above */ >> + { "hle", "on" }, >> + { "rtm", "on" }, >> { "arch-capabilities", "on" }, >> { "rdctl-no", "on" }, >> { "ibrs-all", "on" }, >
>-----Original Message----- >From: Li, Xiaoyao <xiaoyao.li@intel.com> >Sent: Friday, July 15, 2022 9:14 AM >To: Paolo Bonzini <pbonzini@redhat.com>; Duan, Zhenzhong ><zhenzhong.duan@intel.com>; qemu-devel@nongnu.org >Cc: ehabkost@redhat.com; Ma, XiangfeiX <xiangfeix.ma@intel.com>; >Christopherson,, Sean <seanjc@google.com> >Subject: Re: [PATCH v2] target/i386: Restore TSX features with taa-no > >On 7/14/2022 3:59 PM, Paolo Bonzini wrote: >> On 7/14/22 07:36, Zhenzhong Duan wrote: >>> On ICX-2S2 host, when run L2 guest with both L1/L2 using >>> Icelake-Server-v3 >>> or above, we got below warning: >>> >>> "warning: host doesn't support requested feature: MSR(10AH).taa-no >>> [bit 8]" >>> >>> This is because L1 KVM doesn't expose taa-no to L2 if RTM is >>> disabled, then starting L2 qemu triggers the warning. >>> >>> Fix it by restoring TSX features in Icelake-Server-v3, which may also >>> help guest performance if host isn't susceptible to TSX Async Abort >>> (TAA) vulnerabilities. >>> >>> Fixes: d965dc35592d ("target/i386: Add ARCH_CAPABILITIES related bits >>> into Icelake-Server CPU model") >>> Tested-by: Xiangfei Ma <xiangfeix.ma@intel.com> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>> --- >>> v2: Rewrite commit message >> >> Why wouldn't the fix be (in an Icelake-Server-v4 model) to remove taa-no? > >Production Icelake silicon should have the taa-no set, that's the reason taa-no >was added in v3 model. > >When taa-no presents, it's safe to bring TSX features back. > >I'm wondering if we need a new version (v7) for this change. Ping. Any further suggestion on which way to go ahead? Thanks Zhenzhong
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 14f681e998cc..25ef972a3eed 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3423,6 +3423,9 @@ static const X86CPUDefinition builtin_x86_defs[] = { { .version = 3, .props = (PropValue[]) { + /* Restore TSX features removed by -v2 above */ + { "hle", "on" }, + { "rtm", "on" }, { "arch-capabilities", "on" }, { "rdctl-no", "on" }, { "ibrs-all", "on" },
On ICX-2S2 host, when run L2 guest with both L1/L2 using Icelake-Server-v3 or above, we got below warning: "warning: host doesn't support requested feature: MSR(10AH).taa-no [bit 8]" This is because L1 KVM doesn't expose taa-no to L2 if RTM is disabled, then starting L2 qemu triggers the warning. Fix it by restoring TSX features in Icelake-Server-v3, which may also help guest performance if host isn't susceptible to TSX Async Abort (TAA) vulnerabilities. Fixes: d965dc35592d ("target/i386: Add ARCH_CAPABILITIES related bits into Icelake-Server CPU model") Tested-by: Xiangfei Ma <xiangfeix.ma@intel.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- v2: Rewrite commit message target/i386/cpu.c | 3 +++ 1 file changed, 3 insertions(+)