| Message ID | 20171212083524.3765-1-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 12, 2017 at 04:35:24PM +0800, Haozhong Zhang wrote: > Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > clflushopt are not exposed in guest cpuid, clwb and clflushopt > instructions in this test case can still succeed without #UD on the > host CPU which has clwb and clflushopt support, though failures with > UD are expected. > > In order to avoid false alarms in such cases, introduce the following > two arguments "has_clwb" and "has_clflushopt" to allow users to > specify whether clwb and clflushopt are supported on the host CPU. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > x86/memory.c | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/x86/memory.c b/x86/memory.c > index cd1eb46..03ff7d3 100644 > --- a/x86/memory.c > +++ b/x86/memory.c > @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) > regs->rip += isize; > } > > +/* > + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > + * clflushopt are not exposed in guest cpuid, clwb and clflushopt > + * instructions in this test case can still succeed without #UD on > + * the host CPU which has clwb and clflushopt support. In order to avoid > + * false alarms in such cases, introduce the following two arguments > + * to allow users to specify whether clwb and clflushopt are supported on > + * the host CPU: > + * - has_clwb: indicates clwb is supported on the host CPU > + * - has_clflushopt: indicates clflushopt is supported on the host CPU > + */ Why not simply use "-cpu host" to make sure the guest CPUID flags match host CPUID? > int main(int ac, char **av) > { > struct cpuid cpuid7, cpuid1; > int xfail; > + int host_has_clwb = 0, host_has_clflushopt = 0; /* 0: unknown */ > + int i; > + > + for (i = 1; i < ac; i++) > + if (!strcmp(av[i], "has_clwb")) > + host_has_clwb = 1; > + else if (!strcmp(av[i], "has_clflushopt")) > + host_has_clflushopt = 1; > > setup_idt(); > handle_exception(UD_VECTOR, handle_ud); > @@ -63,13 +82,19 @@ int main(int ac, char **av) > ud = 0; > /* clflushopt (%rbx): */ > asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" (&target)); > - report_xfail("clflushopt", xfail, ud == 0); > + if (host_has_clflushopt) > + report("clflushopt", ud == 0); > + else > + report_xfail("clflushopt", xfail, ud == 0); > > xfail = !(cpuid7.b & (1U << 24)); /* CLWB */ > ud = 0; > /* clwb (%rbx): */ > asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" (&target)); > - report_xfail("clwb", xfail, ud == 0); > + if (host_has_clwb) > + report("clwb", ud == 0); > + else > + report_xfail("clwb", xfail, ud == 0); > > ud = 0; > /* clwb requires a memory operand, the following is NOT a valid > -- > 2.14.1 >
On 12/18/17 16:36 -0200, Eduardo Habkost wrote: > On Tue, Dec 12, 2017 at 04:35:24PM +0800, Haozhong Zhang wrote: > > Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > clflushopt are not exposed in guest cpuid, clwb and clflushopt > > instructions in this test case can still succeed without #UD on the > > host CPU which has clwb and clflushopt support, though failures with > > UD are expected. > > > > In order to avoid false alarms in such cases, introduce the following > > two arguments "has_clwb" and "has_clflushopt" to allow users to > > specify whether clwb and clflushopt are supported on the host CPU. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > x86/memory.c | 29 +++++++++++++++++++++++++++-- > > 1 file changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/x86/memory.c b/x86/memory.c > > index cd1eb46..03ff7d3 100644 > > --- a/x86/memory.c > > +++ b/x86/memory.c > > @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) > > regs->rip += isize; > > } > > > > +/* > > + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > + * clflushopt are not exposed in guest cpuid, clwb and clflushopt > > + * instructions in this test case can still succeed without #UD on > > + * the host CPU which has clwb and clflushopt support. In order to avoid > > + * false alarms in such cases, introduce the following two arguments > > + * to allow users to specify whether clwb and clflushopt are supported on > > + * the host CPU: > > + * - has_clwb: indicates clwb is supported on the host CPU > > + * - has_clflushopt: indicates clflushopt is supported on the host CPU > > + */ > > Why not simply use "-cpu host" to make sure the guest CPUID flags > match host CPUID? > Can I understand that testing these two cases with host/guest CPUID mismatch (specially clwb and clflushopt flags) is invalid? If yes, please ignore this patch. Thanks, Haozhong > > > int main(int ac, char **av) > > { > > struct cpuid cpuid7, cpuid1; > > int xfail; > > + int host_has_clwb = 0, host_has_clflushopt = 0; /* 0: unknown */ > > + int i; > > + > > + for (i = 1; i < ac; i++) > > + if (!strcmp(av[i], "has_clwb")) > > + host_has_clwb = 1; > > + else if (!strcmp(av[i], "has_clflushopt")) > > + host_has_clflushopt = 1; > > > > setup_idt(); > > handle_exception(UD_VECTOR, handle_ud); > > @@ -63,13 +82,19 @@ int main(int ac, char **av) > > ud = 0; > > /* clflushopt (%rbx): */ > > asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" (&target)); > > - report_xfail("clflushopt", xfail, ud == 0); > > + if (host_has_clflushopt) > > + report("clflushopt", ud == 0); > > + else > > + report_xfail("clflushopt", xfail, ud == 0); > > > > xfail = !(cpuid7.b & (1U << 24)); /* CLWB */ > > ud = 0; > > /* clwb (%rbx): */ > > asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" (&target)); > > - report_xfail("clwb", xfail, ud == 0); > > + if (host_has_clwb) > > + report("clwb", ud == 0); > > + else > > + report_xfail("clwb", xfail, ud == 0); > > > > ud = 0; > > /* clwb requires a memory operand, the following is NOT a valid > > -- > > 2.14.1 > > > > -- > Eduardo
On Tue, Dec 19, 2017 at 10:54:16AM +0800, Haozhong Zhang wrote: > On 12/18/17 16:36 -0200, Eduardo Habkost wrote: > > On Tue, Dec 12, 2017 at 04:35:24PM +0800, Haozhong Zhang wrote: > > > Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > > clflushopt are not exposed in guest cpuid, clwb and clflushopt > > > instructions in this test case can still succeed without #UD on the > > > host CPU which has clwb and clflushopt support, though failures with > > > UD are expected. > > > > > > In order to avoid false alarms in such cases, introduce the following > > > two arguments "has_clwb" and "has_clflushopt" to allow users to > > > specify whether clwb and clflushopt are supported on the host CPU. > > > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > > --- > > > x86/memory.c | 29 +++++++++++++++++++++++++++-- > > > 1 file changed, 27 insertions(+), 2 deletions(-) > > > > > > diff --git a/x86/memory.c b/x86/memory.c > > > index cd1eb46..03ff7d3 100644 > > > --- a/x86/memory.c > > > +++ b/x86/memory.c > > > @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) > > > regs->rip += isize; > > > } > > > > > > +/* > > > + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > > + * clflushopt are not exposed in guest cpuid, clwb and clflushopt > > > + * instructions in this test case can still succeed without #UD on > > > + * the host CPU which has clwb and clflushopt support. In order to avoid > > > + * false alarms in such cases, introduce the following two arguments > > > + * to allow users to specify whether clwb and clflushopt are supported on > > > + * the host CPU: > > > + * - has_clwb: indicates clwb is supported on the host CPU > > > + * - has_clflushopt: indicates clflushopt is supported on the host CPU > > > + */ > > > > Why not simply use "-cpu host" to make sure the guest CPUID flags > > match host CPUID? > > > > Can I understand that testing these two cases with host/guest CPUID > mismatch (specially clwb and clflushopt flags) is invalid? If yes, > please ignore this patch. I wouldn't say it's invalid to test what happens when the host and guest CPUID don't match. The question is: is it useful to do so? Are we testing different code paths when we do that? The inability to trigger #UD if the host CPUID includes the flag sounds like a bug/limitation we would like to get rid of as soon as hardware allow us to, and not a feature we need to test for. What's the right way to ensure memory.flat is always tested using "-cpu host"?
On 12/20/17 19:26 -0200, Eduardo Habkost wrote: > On Tue, Dec 19, 2017 at 10:54:16AM +0800, Haozhong Zhang wrote: > > On 12/18/17 16:36 -0200, Eduardo Habkost wrote: > > > On Tue, Dec 12, 2017 at 04:35:24PM +0800, Haozhong Zhang wrote: > > > > Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > > > clflushopt are not exposed in guest cpuid, clwb and clflushopt > > > > instructions in this test case can still succeed without #UD on the > > > > host CPU which has clwb and clflushopt support, though failures with > > > > UD are expected. > > > > > > > > In order to avoid false alarms in such cases, introduce the following > > > > two arguments "has_clwb" and "has_clflushopt" to allow users to > > > > specify whether clwb and clflushopt are supported on the host CPU. > > > > > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > > > --- > > > > x86/memory.c | 29 +++++++++++++++++++++++++++-- > > > > 1 file changed, 27 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/x86/memory.c b/x86/memory.c > > > > index cd1eb46..03ff7d3 100644 > > > > --- a/x86/memory.c > > > > +++ b/x86/memory.c > > > > @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) > > > > regs->rip += isize; > > > > } > > > > > > > > +/* > > > > + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and > > > > + * clflushopt are not exposed in guest cpuid, clwb and clflushopt > > > > + * instructions in this test case can still succeed without #UD on > > > > + * the host CPU which has clwb and clflushopt support. In order to avoid > > > > + * false alarms in such cases, introduce the following two arguments > > > > + * to allow users to specify whether clwb and clflushopt are supported on > > > > + * the host CPU: > > > > + * - has_clwb: indicates clwb is supported on the host CPU > > > > + * - has_clflushopt: indicates clflushopt is supported on the host CPU > > > > + */ > > > > > > Why not simply use "-cpu host" to make sure the guest CPUID flags > > > match host CPUID? > > > > > > > Can I understand that testing these two cases with host/guest CPUID > > mismatch (specially clwb and clflushopt flags) is invalid? If yes, > > please ignore this patch. > > I wouldn't say it's invalid to test what happens when the host > and guest CPUID don't match. The question is: is it useful to do > so? Are we testing different code paths when we do that? > No, VMX cannot intercept guest clwb and clflushopt, so no KVM code path is involved when guest executes those two instructions. > The inability to trigger #UD if the host CPUID includes the flag > sounds like a bug/limitation we would like to get rid of as soon > as hardware allow us to, and not a feature we need to test for. > It's more the inability of VMX, which cannot intercept clwb and clflushopt. > What's the right way to ensure memory.flat is always tested using > "-cpu host"? > I think so, at least '-cpu host' can mitigate the hardware inability. Haozhong
This isn't anything new. The same thing happened with ADX instructions on Broadwell, MOVBE on Haswell, F16C instructions on Ivebridge, etc. On Wed, Dec 20, 2017 at 5:08 PM, Haozhong Zhang <haozhong.zhang@intel.com> wrote: > On 12/20/17 19:26 -0200, Eduardo Habkost wrote: >> On Tue, Dec 19, 2017 at 10:54:16AM +0800, Haozhong Zhang wrote: >> > On 12/18/17 16:36 -0200, Eduardo Habkost wrote: >> > > On Tue, Dec 12, 2017 at 04:35:24PM +0800, Haozhong Zhang wrote: >> > > > Intel VMX cannot intercept guest clwb and clflushopt. When clwb and >> > > > clflushopt are not exposed in guest cpuid, clwb and clflushopt >> > > > instructions in this test case can still succeed without #UD on the >> > > > host CPU which has clwb and clflushopt support, though failures with >> > > > UD are expected. >> > > > >> > > > In order to avoid false alarms in such cases, introduce the following >> > > > two arguments "has_clwb" and "has_clflushopt" to allow users to >> > > > specify whether clwb and clflushopt are supported on the host CPU. >> > > > >> > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> > > > --- >> > > > x86/memory.c | 29 +++++++++++++++++++++++++++-- >> > > > 1 file changed, 27 insertions(+), 2 deletions(-) >> > > > >> > > > diff --git a/x86/memory.c b/x86/memory.c >> > > > index cd1eb46..03ff7d3 100644 >> > > > --- a/x86/memory.c >> > > > +++ b/x86/memory.c >> > > > @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) >> > > > regs->rip += isize; >> > > > } >> > > > >> > > > +/* >> > > > + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and >> > > > + * clflushopt are not exposed in guest cpuid, clwb and clflushopt >> > > > + * instructions in this test case can still succeed without #UD on >> > > > + * the host CPU which has clwb and clflushopt support. In order to avoid >> > > > + * false alarms in such cases, introduce the following two arguments >> > > > + * to allow users to specify whether clwb and clflushopt are supported on >> > > > + * the host CPU: >> > > > + * - has_clwb: indicates clwb is supported on the host CPU >> > > > + * - has_clflushopt: indicates clflushopt is supported on the host CPU >> > > > + */ >> > > >> > > Why not simply use "-cpu host" to make sure the guest CPUID flags >> > > match host CPUID? >> > > >> > >> > Can I understand that testing these two cases with host/guest CPUID >> > mismatch (specially clwb and clflushopt flags) is invalid? If yes, >> > please ignore this patch. >> >> I wouldn't say it's invalid to test what happens when the host >> and guest CPUID don't match. The question is: is it useful to do >> so? Are we testing different code paths when we do that? >> > > No, VMX cannot intercept guest clwb and clflushopt, so no KVM code > path is involved when guest executes those two instructions. > >> The inability to trigger #UD if the host CPUID includes the flag >> sounds like a bug/limitation we would like to get rid of as soon >> as hardware allow us to, and not a feature we need to test for. >> > > It's more the inability of VMX, which cannot intercept clwb and > clflushopt. > >> What's the right way to ensure memory.flat is always tested using >> "-cpu host"? >> > > I think so, at least '-cpu host' can mitigate the hardware inability. > > Haozhong
diff --git a/x86/memory.c b/x86/memory.c index cd1eb46..03ff7d3 100644 --- a/x86/memory.c +++ b/x86/memory.c @@ -23,10 +23,29 @@ static void handle_ud(struct ex_regs *regs) regs->rip += isize; } +/* + * Intel VMX cannot intercept guest clwb and clflushopt. When clwb and + * clflushopt are not exposed in guest cpuid, clwb and clflushopt + * instructions in this test case can still succeed without #UD on + * the host CPU which has clwb and clflushopt support. In order to avoid + * false alarms in such cases, introduce the following two arguments + * to allow users to specify whether clwb and clflushopt are supported on + * the host CPU: + * - has_clwb: indicates clwb is supported on the host CPU + * - has_clflushopt: indicates clflushopt is supported on the host CPU + */ int main(int ac, char **av) { struct cpuid cpuid7, cpuid1; int xfail; + int host_has_clwb = 0, host_has_clflushopt = 0; /* 0: unknown */ + int i; + + for (i = 1; i < ac; i++) + if (!strcmp(av[i], "has_clwb")) + host_has_clwb = 1; + else if (!strcmp(av[i], "has_clflushopt")) + host_has_clflushopt = 1; setup_idt(); handle_exception(UD_VECTOR, handle_ud); @@ -63,13 +82,19 @@ int main(int ac, char **av) ud = 0; /* clflushopt (%rbx): */ asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" (&target)); - report_xfail("clflushopt", xfail, ud == 0); + if (host_has_clflushopt) + report("clflushopt", ud == 0); + else + report_xfail("clflushopt", xfail, ud == 0); xfail = !(cpuid7.b & (1U << 24)); /* CLWB */ ud = 0; /* clwb (%rbx): */ asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" (&target)); - report_xfail("clwb", xfail, ud == 0); + if (host_has_clwb) + report("clwb", ud == 0); + else + report_xfail("clwb", xfail, ud == 0); ud = 0; /* clwb requires a memory operand, the following is NOT a valid
Intel VMX cannot intercept guest clwb and clflushopt. When clwb and clflushopt are not exposed in guest cpuid, clwb and clflushopt instructions in this test case can still succeed without #UD on the host CPU which has clwb and clflushopt support, though failures with UD are expected. In order to avoid false alarms in such cases, introduce the following two arguments "has_clwb" and "has_clflushopt" to allow users to specify whether clwb and clflushopt are supported on the host CPU. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- x86/memory.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)