[PATCH] wifi: ath11k: workaround to use VMs

[PATCH] wifi: ath11k: workaround to use VMs

[Jose Ignacio Tornos Martinez]

Currently, this driver is not working when the device is handled in a
Virtual Machine (PCI pass-through), as it was already reported here:
https://lore.kernel.org/all/fc6bd06f-d52b-4dee-ab1b-4bb845cc0b95@quicinc.com/T/
Baochen Qiang focused the problem and described how to have it working
for a specific real MSI vector from host that needs to be used in VM too.
And this value, as it was commented, can change.

The problem seems complex to me and I don't know if there is any easy way
to solve it. Meanwhile and using the information from Baochen Qiang,
since the use of VMs is very interesting for testing procedures,
I would like to just add this workaround that consists on adding a
parameter to pass the real MSI vector from host to the VM. In that way,
checking the 'lscpi' command output from host, it could be handled manually
or with some user tool in order to have the VM with the driver working.
Of course, if this parameter is not configured (zero value and default),
we will have the same behavior as always.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/wireless/ath/ath11k/pci.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index be9d2c69cc41..0e322956b10f 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -31,6 +31,11 @@
 
 #define TCSR_SOC_HW_SUB_VER	0x1910010
 
+static ulong ath11k_host_msi_addr = 0;
+module_param_named(host_msi_addr, ath11k_host_msi_addr, ulong, 0644);
+MODULE_PARM_DESC(host_msi_addr,
+		 "Workaround to configure the MSI address that is used from host in order to be used in VM");
+
 static const struct pci_device_id ath11k_pci_id_table[] = {
 	{ PCI_VDEVICE(QCOM, QCA6390_DEVICE_ID) },
 	{ PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) },
@@ -443,6 +448,18 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
 
 	ath11k_pci_msi_disable(ab_pci);
 
+	if (ath11k_host_msi_addr) {
+		ab_pci->ab->pci.msi.ep_base_data = 0;
+		ab->pci.msi.addr_hi = (u32)(ath11k_host_msi_addr >> 32);
+		ab->pci.msi.addr_lo = (u32)(ath11k_host_msi_addr & 0xffffffff);
+
+		ath11k_dbg(ab, ATH11K_DBG_PCI, "msi addr hi 0x%x lo 0x%x base data is %d\n",
+			   ab->pci.msi.addr_hi,
+			   ab->pci.msi.addr_lo,
+			   ab->pci.msi.ep_base_data);
+		return 0;
+	}
+
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
 		ath11k_err(ab, "msi_desc is NULL!\n");
@@ -482,6 +499,9 @@ static int ath11k_pci_config_msi_data(struct ath11k_pci *ab_pci)
 {
 	struct msi_desc *msi_desc;
 
+	if (ath11k_host_msi_addr)
+		return 0;
+
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
 		ath11k_err(ab_pci->ab, "msi_desc is NULL!\n");
-- 
2.44.0

Re: [PATCH] wifi: ath11k: workaround to use VMs

[Jeff Johnson]

On 3/21/2024 10:23 AM, Jose Ignacio Tornos Martinez wrote:
> Currently, this driver is not working when the device is handled in a
> Virtual Machine (PCI pass-through), as it was already reported here:
> https://lore.kernel.org/all/fc6bd06f-d52b-4dee-ab1b-4bb845cc0b95@quicinc.com/T/
> Baochen Qiang focused the problem and described how to have it working
> for a specific real MSI vector from host that needs to be used in VM too.
> And this value, as it was commented, can change.
> 
> The problem seems complex to me and I don't know if there is any easy way
> to solve it. Meanwhile and using the information from Baochen Qiang,
> since the use of VMs is very interesting for testing procedures,
> I would like to just add this workaround that consists on adding a
> parameter to pass the real MSI vector from host to the VM. In that way,
> checking the 'lscpi' command output from host, it could be handled manually
> or with some user tool in order to have the VM with the driver working.
> Of course, if this parameter is not configured (zero value and default),
> we will have the same behavior as always.
> 
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/wireless/ath/ath11k/pci.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index be9d2c69cc41..0e322956b10f 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -31,6 +31,11 @@
>  
>  #define TCSR_SOC_HW_SUB_VER	0x1910010
>  
> +static ulong ath11k_host_msi_addr = 0;
> +module_param_named(host_msi_addr, ath11k_host_msi_addr, ulong, 0644);
> +MODULE_PARM_DESC(host_msi_addr,
> +		 "Workaround to configure the MSI address that is used from host in order to be used in VM");
> +
>  static const struct pci_device_id ath11k_pci_id_table[] = {
>  	{ PCI_VDEVICE(QCOM, QCA6390_DEVICE_ID) },
>  	{ PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) },
> @@ -443,6 +448,18 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
>  
>  	ath11k_pci_msi_disable(ab_pci);
>  
> +	if (ath11k_host_msi_addr) {
> +		ab_pci->ab->pci.msi.ep_base_data = 0;
> +		ab->pci.msi.addr_hi = (u32)(ath11k_host_msi_addr >> 32);
> +		ab->pci.msi.addr_lo = (u32)(ath11k_host_msi_addr & 0xffffffff);
> +
> +		ath11k_dbg(ab, ATH11K_DBG_PCI, "msi addr hi 0x%x lo 0x%x base data is %d\n",
> +			   ab->pci.msi.addr_hi,
> +			   ab->pci.msi.addr_lo,
> +			   ab->pci.msi.ep_base_data);
> +		return 0;
> +	}
> +
>  	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>  	if (!msi_desc) {
>  		ath11k_err(ab, "msi_desc is NULL!\n");
> @@ -482,6 +499,9 @@ static int ath11k_pci_config_msi_data(struct ath11k_pci *ab_pci)
>  {
>  	struct msi_desc *msi_desc;
>  
> +	if (ath11k_host_msi_addr)
> +		return 0;
> +
>  	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>  	if (!msi_desc) {
>  		ath11k_err(ab_pci->ab, "msi_desc is NULL!\n");

+ kernel@quicinc.com to make sure the Qualcomm VM experts are aware of this
issue and to see if they have any additional suggestions.

Re: [PATCH] wifi: ath11k: workaround to use VMs

[Baochen Qiang]

On 3/22/2024 1:23 AM, Jose Ignacio Tornos Martinez wrote:
> Currently, this driver is not working when the device is handled in a
> Virtual Machine (PCI pass-through), as it was already reported here:
> https://lore.kernel.org/all/fc6bd06f-d52b-4dee-ab1b-4bb845cc0b95@quicinc.com/T/
> Baochen Qiang focused the problem and described how to have it working
> for a specific real MSI vector from host that needs to be used in VM too.
> And this value, as it was commented, can change.
> 
> The problem seems complex to me and I don't know if there is any easy way
> to solve it. Meanwhile and using the information from Baochen Qiang,
> since the use of VMs is very interesting for testing procedures,
> I would like to just add this workaround that consists on adding a
> parameter to pass the real MSI vector from host to the VM. In that way,
> checking the 'lscpi' command output from host, it could be handled manually
> or with some user tool in order to have the VM with the driver working.
> Of course, if this parameter is not configured (zero value and default),
> we will have the same behavior as always.
> 
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>   drivers/net/wireless/ath/ath11k/pci.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index be9d2c69cc41..0e322956b10f 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -31,6 +31,11 @@
>   
>   #define TCSR_SOC_HW_SUB_VER	0x1910010
>   
> +static ulong ath11k_host_msi_addr = 0;
> +module_param_named(host_msi_addr, ath11k_host_msi_addr, ulong, 0644);
> +MODULE_PARM_DESC(host_msi_addr,
> +		 "Workaround to configure the MSI address that is used from host in order to be used in VM");
MSI vector contains two parts: the address and the data. So you need to 
add a parameter for MSI data as well.

> +
>   static const struct pci_device_id ath11k_pci_id_table[] = {
>   	{ PCI_VDEVICE(QCOM, QCA6390_DEVICE_ID) },
>   	{ PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) },
> @@ -443,6 +448,18 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
>   
>   	ath11k_pci_msi_disable(ab_pci);
>   
> +	if (ath11k_host_msi_addr) {
> +		ab_pci->ab->pci.msi.ep_base_data = 0;
Here, I guess you are assigning 0 because I gave it in my example. But 
it is not always that, it changes from machine to machine, configuration 
to configuration, and even time to time.

The right way should be to use the MSI data parameter mentioned above.

> +		ab->pci.msi.addr_hi = (u32)(ath11k_host_msi_addr >> 32);
> +		ab->pci.msi.addr_lo = (u32)(ath11k_host_msi_addr & 0xffffffff);
> +
> +		ath11k_dbg(ab, ATH11K_DBG_PCI, "msi addr hi 0x%x lo 0x%x base data is %d\n",
> +			   ab->pci.msi.addr_hi,
> +			   ab->pci.msi.addr_lo,
> +			   ab->pci.msi.ep_base_data);
> +		return 0;
> +	}
> +
>   	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>   	if (!msi_desc) {
>   		ath11k_err(ab, "msi_desc is NULL!\n");
> @@ -482,6 +499,9 @@ static int ath11k_pci_config_msi_data(struct ath11k_pci *ab_pci)
>   {
>   	struct msi_desc *msi_desc;
>   
> +	if (ath11k_host_msi_addr)
> +		return 0;
> +
>   	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>   	if (!msi_desc) {
>   		ath11k_err(ab_pci->ab, "msi_desc is NULL!\n");

Re: [PATCH] wifi: ath11k: workaround to use VMs

[Baochen Qiang]

On 3/22/2024 4:13 AM, Jeff Johnson wrote:
> On 3/21/2024 10:23 AM, Jose Ignacio Tornos Martinez wrote:
>> Currently, this driver is not working when the device is handled in a
>> Virtual Machine (PCI pass-through), as it was already reported here:
>> https://lore.kernel.org/all/fc6bd06f-d52b-4dee-ab1b-4bb845cc0b95@quicinc.com/T/
>> Baochen Qiang focused the problem and described how to have it working
>> for a specific real MSI vector from host that needs to be used in VM too.
>> And this value, as it was commented, can change.
>>
>> The problem seems complex to me and I don't know if there is any easy way
>> to solve it. Meanwhile and using the information from Baochen Qiang,
>> since the use of VMs is very interesting for testing procedures,
>> I would like to just add this workaround that consists on adding a
>> parameter to pass the real MSI vector from host to the VM. In that way,
>> checking the 'lscpi' command output from host, it could be handled manually
>> or with some user tool in order to have the VM with the driver working.
>> Of course, if this parameter is not configured (zero value and default),
>> we will have the same behavior as always.
>>
>> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
>> ---
>>   drivers/net/wireless/ath/ath11k/pci.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
>> index be9d2c69cc41..0e322956b10f 100644
>> --- a/drivers/net/wireless/ath/ath11k/pci.c
>> +++ b/drivers/net/wireless/ath/ath11k/pci.c
>> @@ -31,6 +31,11 @@
>>   
>>   #define TCSR_SOC_HW_SUB_VER	0x1910010
>>   
>> +static ulong ath11k_host_msi_addr = 0;
>> +module_param_named(host_msi_addr, ath11k_host_msi_addr, ulong, 0644);
>> +MODULE_PARM_DESC(host_msi_addr,
>> +		 "Workaround to configure the MSI address that is used from host in order to be used in VM");
>> +
>>   static const struct pci_device_id ath11k_pci_id_table[] = {
>>   	{ PCI_VDEVICE(QCOM, QCA6390_DEVICE_ID) },
>>   	{ PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) },
>> @@ -443,6 +448,18 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
>>   
>>   	ath11k_pci_msi_disable(ab_pci);
>>   
>> +	if (ath11k_host_msi_addr) {
>> +		ab_pci->ab->pci.msi.ep_base_data = 0;
>> +		ab->pci.msi.addr_hi = (u32)(ath11k_host_msi_addr >> 32);
>> +		ab->pci.msi.addr_lo = (u32)(ath11k_host_msi_addr & 0xffffffff);
>> +
>> +		ath11k_dbg(ab, ATH11K_DBG_PCI, "msi addr hi 0x%x lo 0x%x base data is %d\n",
>> +			   ab->pci.msi.addr_hi,
>> +			   ab->pci.msi.addr_lo,
>> +			   ab->pci.msi.ep_base_data);
>> +		return 0;
>> +	}
>> +
>>   	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>>   	if (!msi_desc) {
>>   		ath11k_err(ab, "msi_desc is NULL!\n");
>> @@ -482,6 +499,9 @@ static int ath11k_pci_config_msi_data(struct ath11k_pci *ab_pci)
>>   {
>>   	struct msi_desc *msi_desc;
>>   
>> +	if (ath11k_host_msi_addr)
>> +		return 0;
>> +
>>   	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
>>   	if (!msi_desc) {
>>   		ath11k_err(ab_pci->ab, "msi_desc is NULL!\n");
> 
> + kernel@quicinc.com to make sure the Qualcomm VM experts are aware of this
> issue and to see if they have any additional suggestions.
> 
If limit the fix/WAR to be within ath11k, not touching vfio or any other 
related module, I was imagining that if hardware could map the standard 
MSI registers in config space to some private MMIO registers so VM 
ath11k could read, or that if firmware could read them and send to VM 
ath11k via QMI interface? don't know if it is feasible to hardware/firmware.
> 

Re: [PATCH] wifi: ath11k: workaround to use VMs

[Jose Ignacio Tornos Martinez]

Thank you for your comments.
I don't have seen any change in MSI vector data but if it can be different from zero I will complete it.

Best regards
José Ignacio

[PATCH V2] wifi: ath11k: workaround to use VMs

[Jose Ignacio Tornos Martinez]

Currently, this driver is not working when the device is handled in a
Virtual Machine (PCI pass-through), as it was already reported here:
https://lore.kernel.org/all/fc6bd06f-d52b-4dee-ab1b-4bb845cc0b95@quicinc.com/T/
Baochen Qiang focused the problem and described how to have it working
for a specific real MSI vector from host that needs to be used in VM too.
And this value, as it was commented, can change.

The problem seems complex to me and I don't know if there is any easy way
to solve it (with no more information, not hardware/firmware related help
or VMM action).
Meanwhile and using the information from Baochen Qiang, since the use of
VMs is very interesting for testing procedures, I would like to include
this workaround that consists on adding two parameters to pass the real MSI
vector address and data from host to the VM.
In that way, checking the 'lscpi' command output from host, it could be
handled manually or with some user tool in order to have the VM with the
driver working.
Of course, if the workaround is not used, that is if MSI vector address
parameter is not configured (zero value and default), we will have the same
behavior as always.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- Add parameter for msi vector data as Baochen Qiang suggests.

 drivers/net/wireless/ath/ath11k/pci.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index be9d2c69cc41..4c84208dcf5d 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -31,6 +31,15 @@
 
 #define TCSR_SOC_HW_SUB_VER	0x1910010
 
+static ulong ath11k_host_msi_vector_addr = 0;
+module_param_named(host_msi_vector_addr, ath11k_host_msi_vector_addr, ulong, 0644);
+MODULE_PARM_DESC(host_msi_vector_addr,
+		 "Workaround to configure the MSI vector address that is used from host in order to be used in VM");
+static uint ath11k_host_msi_vector_data = 0;
+module_param_named(host_msi_vector_data, ath11k_host_msi_vector_data, uint, 0644);
+MODULE_PARM_DESC(host_msi_vector_data,
+		 "Workaround to configure the MSI vector data that is used from host in order to be used in VM");
+
 static const struct pci_device_id ath11k_pci_id_table[] = {
 	{ PCI_VDEVICE(QCOM, QCA6390_DEVICE_ID) },
 	{ PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) },
@@ -443,6 +452,18 @@ static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
 
 	ath11k_pci_msi_disable(ab_pci);
 
+	if (ath11k_host_msi_vector_addr) {
+		ab_pci->ab->pci.msi.ep_base_data = ath11k_host_msi_vector_data;
+		ab->pci.msi.addr_hi = (u32)(ath11k_host_msi_vector_addr >> 32);
+		ab->pci.msi.addr_lo = (u32)(ath11k_host_msi_vector_addr & 0xffffffff);
+
+		ath11k_dbg(ab, ATH11K_DBG_PCI, "msi addr hi 0x%x lo 0x%x base data is %d\n",
+			   ab->pci.msi.addr_hi,
+			   ab->pci.msi.addr_lo,
+			   ab->pci.msi.ep_base_data);
+		return 0;
+	}
+
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
 		ath11k_err(ab, "msi_desc is NULL!\n");
@@ -482,6 +503,9 @@ static int ath11k_pci_config_msi_data(struct ath11k_pci *ab_pci)
 {
 	struct msi_desc *msi_desc;
 
+	if (ath11k_host_msi_vector_addr)
+		return 0;
+
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
 		ath11k_err(ab_pci->ab, "msi_desc is NULL!\n");
-- 
2.44.0