Re: [Xen-devel] [PATCH v2] x86/mce: handle reserved domain ID in XEN_MC_msrinject

2016-05-27 Thread Haozhong Zhang 
On 05/27/16 06:27, Jan Beulich wrote:
> >>> On 27.05.16 at 14:13,  wrote:
> > --- a/tools/tests/mce-test/tools/xen-mceinj.c
> > +++ b/tools/tests/mce-test/tools/xen-mceinj.c
> > @@ -317,7 +317,9 @@ static int inject_mci_addr(xc_interface *xc_handle,
> > domid_t domid)
> >  {
> >  return add_msr_bank_intpose(xc_handle, cpu_nr,
> > -MC_MSRINJ_F_INTERPOSE | MC_MSRINJ_F_GPADDR,
> > +MC_MSRINJ_F_INTERPOSE |
> > +(domid > DOMID_FIRST_RESERVED ?
> 
> Simply by the name of it I would say this ought to be >=. However,
> DOMID_FIRST_RESERVED == DOMID_SELF, so the > seems right
> here (on the hypervisor side it should be changed though). But for
> clarity I would suggest either adding a respective comment or
> indeed using
> 
>   domid >= DOMID_FIRST_RESERVED && domid != DOMID_SELF
> 
> (which I would hope the compiler can fold).
> 

I'll change on both sides in the next version.

Thanks,
Haozhong

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] x86/mce: handle reserved domain ID in XEN_MC_msrinject

2016-05-27 Thread Jan Beulich 
>>> On 27.05.16 at 14:13,  wrote:
> --- a/tools/tests/mce-test/tools/xen-mceinj.c
> +++ b/tools/tests/mce-test/tools/xen-mceinj.c
> @@ -317,7 +317,9 @@ static int inject_mci_addr(xc_interface *xc_handle,
> domid_t domid)
>  {
>  return add_msr_bank_intpose(xc_handle, cpu_nr,
> -MC_MSRINJ_F_INTERPOSE | MC_MSRINJ_F_GPADDR,
> +MC_MSRINJ_F_INTERPOSE |
> +(domid > DOMID_FIRST_RESERVED ?

Simply by the name of it I would say this ought to be >=. However,
DOMID_FIRST_RESERVED == DOMID_SELF, so the > seems right
here (on the hypervisor side it should be changed though). But for
clarity I would suggest either adding a respective comment or
indeed using

domid >= DOMID_FIRST_RESERVED && domid != DOMID_SELF

(which I would hope the compiler can fold).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] x86/mce: handle reserved domain ID in XEN_MC_msrinject

2016-05-27 Thread Haozhong Zhang 
Commit 26646f3 "x86/mce: translate passed-in GPA to host machine
address" and commit 4ddf474 "tools/xen-mceinj: Pass in GPA when
injecting through MSR_MCI_ADDR" forgot to consider reserved domain
ID and mistakenly add MC_MSRINJ_F_GPADDR flag for them, which in turn
causes bug reported by
http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02640.html.

This patch removes MC_MSRINK_F_GPADDR flag and check this when injecting
to domain ID larger than DOMID_FIRST_RESERVED, and treats the passed-in
address as host machine address.

Signed-off-by: Haozhong Zhang 
---
This is v2 of 
http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02534.html.

Changes in v2:
 * Consider all reserved domain IDs rather than just DOMID_XEN.
---
 tools/tests/mce-test/tools/xen-mceinj.c |  4 +++-
 xen/arch/x86/cpu/mcheck/mce.c   | 14 +++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/tests/mce-test/tools/xen-mceinj.c 
b/tools/tests/mce-test/tools/xen-mceinj.c
index 061ec7c..055db7b 100644
--- a/tools/tests/mce-test/tools/xen-mceinj.c
+++ b/tools/tests/mce-test/tools/xen-mceinj.c
@@ -317,7 +317,9 @@ static int inject_mci_addr(xc_interface *xc_handle,
domid_t domid)
 {
 return add_msr_bank_intpose(xc_handle, cpu_nr,
-MC_MSRINJ_F_INTERPOSE | MC_MSRINJ_F_GPADDR,
+MC_MSRINJ_F_INTERPOSE |
+(domid > DOMID_FIRST_RESERVED ?
+ 0 : MC_MSRINJ_F_GPADDR),
 MCi_type_ADDR, bank, val, domid);
 }
 
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index cc446eb..711a97c 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1427,6 +1427,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 
 if ( mc_msrinject->mcinj_flags & MC_MSRINJ_F_GPADDR )
 {
+domid_t domid;
 struct domain *d;
 struct mcinfo_msr *msr;
 unsigned int i;
@@ -1434,10 +1435,17 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 unsigned long gfn, mfn;
 p2m_type_t t;
 
-d = get_domain_by_id(mc_msrinject->mcinj_domid);
+domid = (mc_msrinject->mcinj_domid == DOMID_SELF) ?
+current->domain->domain_id : mc_msrinject->mcinj_domid;
+if ( domid > DOMID_FIRST_RESERVED )
+return x86_mcerr("do_mca inject: incompatible flag "
+ "MC_MSRINJ_F_GPADDR with domain %d",
+ -EINVAL, domid);
+
+d = get_domain_by_id(domid);
 if ( d == NULL )
 return x86_mcerr("do_mca inject: bad domain id %d",
- -EINVAL, mc_msrinject->mcinj_domid);
+ -EINVAL, domid);
 
 for ( i = 0, msr = _msrinject->mcinj_msr[0];
   i < mc_msrinject->mcinj_count;
@@ -1452,7 +1460,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 put_gfn(d, gfn);
 put_domain(d);
 return x86_mcerr("do_mca inject: bad gfn %#lx of domain 
%d",
- -EINVAL, gfn, mc_msrinject->mcinj_domid);
+ -EINVAL, gfn, domid);
 }
 
 msr->value = pfn_to_paddr(mfn) | (gaddr & (PAGE_SIZE - 1));
-- 
2.8.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel