Re: [Xen-devel] [PATCH] Xen/MCE: adjust for future new vMCE model

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.07.12 at 05:30, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> @@ -68,7 +74,7 @@
>  
>  int vmce_restore_vcpu(struct vcpu *v, uint64_t caps)
>  {
> -    if ( caps & ~g_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
> +    if ( caps & ~g_mcg_cap & ~MCG_CAP_COUNT )
>      {
>          dprintk(XENLOG_G_ERR, "%s restore: unsupported MCA capabilities"
>                  " %#" PRIx64 " for d%d:v%u (supported: %#Lx)\n",
> @@ -77,6 +83,12 @@
>          return -EPERM;
>      }
>  
> +    if ( (caps & MCG_CAP_COUNT) != GUEST_BANK_NUM )
> +    {
> +        dprintk(XENLOG_G_ERR, "Bank number inconsistency\n");
> +        return -EPERM;
> +    }
> +
>      v->arch.mcg_cap = caps;
>      return 0;
>  }

These two changes, as pointed out before, need some further
thought and tweaking: As I said earlier, we are already shipping
the code in its prior form, so outright rejecting MCG_CTL_P set
and non-default bank counts is not a proper solution. Warning
about them being in an unsupported state is certainly acceptable.

And I think the guest visible MCG_CAP value also shouldn't
change across migration, so tolerating (but ignoring) a higher
bank count seems necessary. Not sure what to do when the
bank count is lower (0 or 1) - for 1, all communication to the
guest should probably go through bank 0, while 0 should
probably disable vMCE  for that vCPU.

Further I just noticed that you don't touch fill_vmsr_data() at
all (sending patches created with diff's -p option or equivalent
helps spotting where individual changes belong), yet that
function uses the hardware bank number to fill the struct
bank_entry. With the intended concept, the "bank" member
of that structure can probably go away altogether.

Jan


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