[PATCH 09/11][jammy/linux-azure] PCI: hv: Fix hv_arch_irq_unmask() for multi-MSI

[Tim Gardner]

On 7/14/22 03:18, Bartlomiej Zolnierkiewicz wrote:
> On Wed, Jul 13, 2022 at 2:58 PM Tim Gardner <tim.gardner at canonical.com> wrote:
>>
>> From: Jeffrey Hugo <quic_jhugo at quicinc.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1981577
>>
>> In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first
>> MSI of the N allocated.  This is because only the first msi_desc is cached
>> and it is shared by all the MSIs of the multi-MSI block.  This means that
>> hv_arch_irq_unmask() gets the correct address, but the wrong data (always
>> 0).
>>
>> This can break MSIs.
>>
>> Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0.
>>
>> hv_arch_irq_unmask() is called on MSI0.  It uses a hypercall to configure
>> the MSI address and data (0) to vector 34 of CPU0.  This is correct.  Then
>> hv_arch_irq_unmask is called on MSI1.  It uses another hypercall to
>> configure the MSI address and data (0) to vector 33 of CPU0.  This is
>> wrong, and results in both MSI0 and MSI1 being routed to vector 33.  Linux
>> will observe extra instances of MSI1 and no instances of MSI0 despite the
>> endpoint device behaving correctly.
>>
>> For the multi-MSI case, we need unique address and data info for each MSI,
>> but the cached msi_desc does not provide that.  However, that information
>> can be gotten from the int_desc cached in the chip_data by
>> compose_msi_msg().  Fix the multi-MSI case to use that cached information
>> instead.  Since hv_set_msi_entry_from_desc() is no longer applicable,
>> remove it.
>>
>> Signed-off-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
>> Reviewed-by: Michael Kelley <mikelley at microsoft.com>
>> Link: https://lore.kernel.org/r/1651068453-29588-1-git-send-email-quic_jhugo@quicinc.com
>> Signed-off-by: Wei Liu <wei.liu at kernel.org>
>> (cherry picked from commit 455880dfe292a2bdd3b4ad6a107299fce610e64b)
>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
>> ---
>>   drivers/pci/controller/pci-hyperv.c | 12 ++++--------
>>   1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
>> index 2244338a56ba..317d8d8e52e9 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -605,13 +605,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data)
>>          return cfg->vector;
>>   }
>>
>> -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
>> -                                      struct msi_desc *msi_desc)
>> -{
>> -       msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
>> -       msi_entry->data.as_uint32 = msi_desc->msg.data;
>> -}
>> -
> 
> Don't we also need to backport commit
> 22ef7ee3eeb2a41e07f611754ab9a2663232fedf ("PCI: hv: Remove unused
> hv_set_msi_entry_from_desc()") after applying patch #8 [ which is
> commit d06957d7a6929e6a4aa959cb59d66f0c095fc974 ("PCI: hv: Avoid the
> retarget interrupt hypercall in irq_unmask() on ARM64") ] and before
> applying this patch?
> 

22ef7ee3eeb2a41e07f611754ab9a2663232fedf ("PCI: hv: Remove unused 
hv_set_msi_entry_from_desc()") does apply, but its not important for our 
build since it doesn't fail with an unused function warning. I can 
certainly insert it when I push the patches.

rtg

-- 
-----------
Tim Gardner
Canonical, Inc