Re: [Xen-devel] [PATCH v4] x86: properly handle MSI-X unmask operation from guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.11.13 at 02:40, "Wu, Feng" <feng.wu@xxxxxxxxx> wrote:
>> > +struct pending_msix_unmask_info
>> > +{
>> > +    unsigned long ctrl_address;
>> > +    bool_t valid;
>> > +};
>> > +
>> >  struct arch_vcpu
>> >  {
>> >      /*
>> > @@ -439,6 +445,8 @@ struct arch_vcpu
>> >
>> >      /* A secondary copy of the vcpu time info. */
>> >      XEN_GUEST_HANDLE(vcpu_time_info_t) time_info_guest;
>> > +
>> > +    struct pending_msix_unmask_info pending_msix_unmask;
>> 
>> I don't think you need a separate boolean here - for one I don't
>> think the address can reasonably be zero, and even if you have
>> the bottom two bits available (as it being 4-byte aligned gets
>> checked before you consume it).
> 
> 
> The boolean variant "valid", which is set in msixtbl_write(), means whether 
> there is a
> msix pending unmask, if there is, just clean this flag and unmask the msix 
> in hardware,
> otherwise we just do nothing. If removing "valid", how can I know whether 
> there is a
> msix pending unmask operation ? Thanks you!

Quoting your patch:

@@ -293,7 +293,11 @@ static int msixtbl_write(struct vcpu *v, unsigned long 
address,

     /* exit to device model if address/data has been modified */
     if ( test_and_clear_bit(nr_entry, &entry->table_flags) )
+    {
+        v->arch.pending_msix_unmask.valid = 1;
+        v->arch.pending_msix_unmask.ctrl_address = address;
         goto out;
+    }

     virt = msixtbl_addr_to_virt(entry, address);
     if ( !virt )

You always set "ctrl_address" together with "valid". Hence, as long
as "ctrl_address" is guaranteed to be non-zero (either a apriori
knowledge, or by setting one of the low to bits, which aren't used
for other purposes), you can then check "ctrl_address" to be non-
zero instead of checking "valid".

Jan


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