Re: [Xen-devel] [PATCH] xen vtd : set msi guest_masked 0 by default

[jzh Chang <changjzh@xxxxxxxxx>]

2016-01-26 20:56 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:

>>> On 26.01.16 at 02:34, <changjzh@xxxxxxxxx> wrote:
> There are some problems when msi guest_masked is set to 1 by default.
> When guest os is windows 2008 r2 server,
> the device(eg X540-AT2 vf) is not initialized correctly.
> Host will always receive message like this :"VF Reset msg received from vf".
> Guest has network connectivity issues,
> and can not correctly receive/send the packet.
> So, guest_masked is set to 0 by default.

You describe a problem and half of your change, but there's no
connection between the two: What is actually wrong with current
behavior (matching the hardware's - MSI-X mask bits are set when
coming out of reset).

> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -512,7 +512,7 @@ void guest_mask_msi_irq(struct irq_desc *desc, bool_t mask)
>
>  static unsigned int startup_msi_irq(struct irq_desc *desc)
>  {
> -    if ( unlikely(!msi_set_mask_bit(desc, 0, !!(desc->status & IRQ_GUEST))) )
> +    if ( unlikely(!msi_set_mask_bit(desc, 0, 0) ))
>          WARN();
>      return 0;
>  }

Whether this part can go under "set ... by default" is highly
questionable. Plus, while this affects MSI and MSI-X, ...

 If irq is owned by guest,in function msi_set_mask_bit():

...

bool_t flag = host || guest; //The flag should be true.

...

 writel(flag, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);

...

PCI device can not generate interrrupt.

windows guest can not change vector_ctrl_mask, guest os get abnormal status of nic.

> @@ -972,7 +972,7 @@ static int msix_capability_init(struct pci_dev *dev,
>          entry->msi_attrib.entry_nr = msi->entry_nr;
>          entry->msi_attrib.maskbit = 1;
>          entry->msi_attrib.host_masked = 1;
> -        entry->msi_attrib.guest_masked = 1;
> +        entry->msi_attrib.guest_masked = 0;
>          entry->msi_attrib.pos = pos;
>          entry->irq = msi->irq;
>          entry->dev = dev;

... this change affect MSI-X only, and doing some guessing from
what you write above I suspect you only really tested one of the
two cases.

So while the change _may_ be necessary, you'll need to do a
better job at explaining why you what you do.

Msi guest_masked is set to 0 in the original code, only msi-x guest_masked is modifed in msix_capability_init() function by patch. 

Jan

This issue appears after commited the variable guest_mask.

Initialization operations of pci device may be changed in windows guest,or Xen need to change the initial state of vtd pci device.

--

Jianzhong,Chang

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