Re: [Xen-devel] [PATCH] x86/MSI: drop local cpumask_t variable from msi_compose_msg()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.10.11 at 14:08, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> On 14/10/2011 11:24, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> The function gets called only during initialization/resume (when no
>> other CPUs are running) or with the IRQ descriptor lock held, so
>> there's no way for the CPU mask to change under its feet.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Keir Fraser <keir@xxxxxxx>
> 
> I wonder whether the cpus_empty() check should be a BUG_ON. Or an ASSERT
> pushed into cpu_mask_to_apicid.

An ASSERT may be reasonable, but simply dropping the check here
may be too - no other code path invoking cpu_mask_to_apicid() has
a similar check.

Jan

>  -- Keir
> 
>> --- a/xen/arch/x86/msi.c
>> +++ b/xen/arch/x86/msi.c
>> @@ -123,18 +123,16 @@ static void msix_put_fixmap(struct pci_d
>>  void msi_compose_msg(struct irq_desc *desc, struct msi_msg *msg)
>>  {
>>      unsigned dest;
>> -    cpumask_t domain;
>>      struct irq_cfg *cfg = desc->chip_data;
>>      int vector = cfg->vector;
>> -    domain = cfg->cpu_mask;
>>  
>> -    if ( cpus_empty( domain ) ) {
>> +    if ( cpus_empty(cfg->cpu_mask) ) {
>>          dprintk(XENLOG_ERR,"%s, compose msi message error!!\n", __func__);
>> -     return;
>> +        return;
>>      }
>>  
>>      if ( vector ) {
>> -        dest = cpu_mask_to_apicid(&domain);
>> +        dest = cpu_mask_to_apicid(&cfg->cpu_mask);
>>  
>>          msg->address_hi = MSI_ADDR_BASE_HI;
>>          msg->address_lo =
>> 
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx 
>> http://lists.xensource.com/xen-devel 




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