WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 4] MCA physical address check when calculate doma

To: "Jinsong Liu" <jinsong.liu@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 4] MCA physical address check when calculate domain
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 09 May 2011 10:16:08 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir.xen@xxxxxxxxx>, Xin Li <xin.li@xxxxxxxxx>, Yunhong Jiang <yunhong.jiang@xxxxxxxxx>
Delivery-date: Mon, 09 May 2011 02:16:42 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <BC00F5384FCFC9499AF06F92E8B78A9E2075FE9CD0@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <BC00F5384FCFC9499AF06F92E8B78A9E2075FE9CD0@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> On 07.05.11 at 22:29, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> MCA physical address check when calculate domain
> 
> Bank addr maybe invalid, or 
> Bank addr maybe physical/memory/linear address or segment offset.
> This patch add mca MCi_STATUS_MISCV/MCi_STATUS_ADDRV check, and add physical 
> address verify,
> so that it work safe when calculate domain.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
> 
> diff -r e4e1efda200f xen/arch/x86/cpu/mcheck/mce.c
> --- a/xen/arch/x86/cpu/mcheck/mce.c   Thu May 05 13:54:29 2011 +0800
> +++ b/xen/arch/x86/cpu/mcheck/mce.c   Fri May 06 13:56:21 2011 +0800
> @@ -151,7 +151,6 @@ static struct mcinfo_bank *mca_init_bank
>                                           struct mc_info *mi, int bank)
>  {
>      struct mcinfo_bank *mib;
> -    uint64_t addr=0, misc = 0;
>  
>      if (!mi)
>          return NULL;
> @@ -170,22 +169,23 @@ static struct mcinfo_bank *mca_init_bank
>      mib->common.size = sizeof (struct mcinfo_bank);
>      mib->mc_bank = bank;
>  
> -    addr = misc = 0;
>      if (mib->mc_status & MCi_STATUS_MISCV)
>          mib->mc_misc = mca_rdmsr(MSR_IA32_MCx_MISC(bank));
>  
>      if (mib->mc_status & MCi_STATUS_ADDRV)
> -    {
>          mib->mc_addr = mca_rdmsr(MSR_IA32_MCx_ADDR(bank));
>  
> -        if (mfn_valid(paddr_to_pfn(mib->mc_addr))) {
> -            struct domain *d;
> +    if ((mib->mc_status & MCi_STATUS_MISCV) &&
> +        (mib->mc_status & MCi_STATUS_ADDRV) &&
> +        ((mib->mc_misc & MCi_MISC_ADDRMOD_MASK) == MCi_MISC_PHYSMOD) && 
> +          mfn_valid(paddr_to_pfn(mib->mc_addr)))
> +    {
> +        struct domain *d;
>  
> -            d = maddr_get_owner(mib->mc_addr);
> -            if (d != NULL && (who == MCA_POLLER ||
> -                              who == MCA_CMCI_HANDLER))
> -                mib->mc_domid = d->domain_id;
> -        }
> +        d = maddr_get_owner(mib->mc_addr);
> +        if (d != NULL && (who == MCA_POLLER ||
> +                          who == MCA_CMCI_HANDLER))
> +            mib->mc_domid = d->domain_id;

This wasn't very logical before, and would probably be good if it
got changed while you re-structure it anyway: Rather than
calling maddr_get_owner() and *then* checking "who", that
check should be added to the other surrounding ones, thus
avoiding the possibly pointless call.

Further I wonder whether there aren't more cases for which the
domain ID could be set, as well as whether the !MISCV case
shouldn't also assume the address to be physical. Wouldn't that
be the case e.g. on older CPUs?

Jan

>      }
>  
>      if (who == MCA_CMCI_HANDLER) {




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel