Re: [Xen-devel] [PATCH v10 1/6] VT-d: Introduce new fields in msi_desc to track binding with guest interrupt

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.17 at 22:21, <chao.gao@xxxxxxxxx> wrote:
> On Wed, Mar 15, 2017 at 10:41:13AM -0600, Jan Beulich wrote:
>>>>> On 15.03.17 at 06:11, <chao.gao@xxxxxxxxx> wrote:
>>> --- a/xen/drivers/passthrough/vtd/intremap.c
>>> +++ b/xen/drivers/passthrough/vtd/intremap.c
>>> @@ -552,11 +552,12 @@ static int msi_msg_to_remap_entry(
>>>      struct msi_desc *msi_desc, struct msi_msg *msg)
>>>  {
>>>      struct iremap_entry *iremap_entry = NULL, *iremap_entries;
>>> -    struct iremap_entry new_ire;
>>> +    struct iremap_entry new_ire = {{0}};
>>
>>Any reason this isn't simple "{ }"?
>>
> 
> I also think '{}' can work. But here is the compiler output:
> intremap.c: In function ‘msi_msg_to_remap_entry’:
> intremap.c:587:12: error: missing braces around initializer 
> [-Werror=missing-braces]
>      struct iremap_entry new_ire = {0};
>             ^
> intremap.c:587:12: error: (near initialization for ‘new_ire.<anonymous>’) 
> [-Werror=missing-braces]

Sure - you've left the 0 there, other than suggested.

>>> @@ -968,59 +927,14 @@ int pi_update_irte(const struct vcpu *v, const struct 
>>> pirq *pirq,
>>>          rc = -ENODEV;
>>>          goto unlock_out;
>>>      }
>>> -
>>> -    pci_dev = msi_desc->dev;
>>> -    if ( !pci_dev )
>>> -    {
>>> -        rc = -ENODEV;
>>> -        goto unlock_out;
>>> -    }
>>> -
>>> -    remap_index = msi_desc->remap_index;
>>> +    msi_desc->pi_desc = pi_desc;
>>> +    msi_desc->gvec = gvec;
>>
>>Am I overlooking something - I can't seem to find any place where these
>>two fields (or at least the former) get cleared again? This may be correct,
>>but if it is the reason wants recording in the commit message.
> 
> IIUC, the current logic is free the whole msi_desc when device deassignment.
> But it is better to clear this two fields explicitly. I will call 
> pi_update_irte()
> with @vcpu=NULL, just like Patch [6/6] when device deassignment.
> Do you think the new added code should be squashed into this one?

Not sure which code specifically you think about.

> Shall I also squash Patch [6/6] to this one? I think it is to fix another 
> flaw.

I haven't got around to look at patch 6 yet.

Jan

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