* Re: [GIT PULL v3] Hyper-V commits for 5.0-rc
From: Kimberly Brown @ 2019-02-23 16:47 UTC (permalink / raw)
To: Sasha Levin
Cc: Greg KH, linux-kernel, linux-hyperv, kys, haiyangz, sthemmin,
linux-kernel, Michael Kelley
In-Reply-To: <20190223155533.GE10616@sasha-vm>
On Sat, Feb 23, 2019 at 10:55:33AM -0500, Sasha Levin wrote:
> On Sat, Feb 23, 2019 at 09:57:15AM +0100, Greg KH wrote:
> > On Fri, Feb 22, 2019 at 10:12:25PM -0500, Sasha Levin wrote:
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA512
> > >
> > > The following changes since commit 8834f5600cf3c8db365e18a3d5cac2c2780c81e5:
> > >
> > > Linux 5.0-rc5 (2019-02-03 13:48:04 -0800)
> > >
> > > are available in the Git repository at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
> > >
> > > for you to fetch changes up to b2946cd86f3c1b5b1262c0ec06068110c2328fe6:
> > >
> > > MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS (2019-02-22 21:46:37 -0500)
> > >
> > > - ----------------------------------------------------------------
> > > Two fixes:
> > >
> > > 1. A fix for a race condition reading sysfs entries while a device is
> > > being added, by Kimberly Brown.
> > >
> > > 2. Update the Hyper-V mailing list to a new one created on
> > > vger.kernel.org, by Haiyang Zhang.
> > >
> > > - ----------------------------------------------------------------
> > > Haiyang Zhang (1):
> > > MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS
> > >
> > > Kimberly Brown (2):
> > > Drivers: hv: vmbus: Change server monitor_pages index to 0
> > > Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set
> >
> > I objected to this last patch when it was posted on the list. The sysfs
> > file should just not be present if the functionality is not there, no
> > need to add the "-EINVAL" logic to it instead.
> >
> > Having a sysfs file that says it can be read, and then rejecting that
> > read with an error is NOT ok.
>
> Hm, I'm sorry but I didn't see an objection on the thread
> (https://lore.kernel.org/lkml/20190122020759.GA4054@ubu-Virtual-Machine/)
> which is why it was sent in like this.
>
> Could you please point me to it so we can get the patch fixed up?
>
The correct link to the "Drivers: hv: vmbus: Return -EINVAL if
monitor_allocated not set" patch thread is:
https://lore.kernel.org/lkml/cover.1549619051.git.kimbrownkd@gmail.com/
Also, this patch doesn't fix a race condition; it fixes a problem with
sysfs returning invalid data.
> --
> Thanks,
> Sasha
^ permalink raw reply
* Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Stephen Hemminger @ 2019-02-23 16:46 UTC (permalink / raw)
To: Haiyang Zhang
Cc: haiyangz, sashal, linux-hyperv, kys, sthemmin, olaf, vkuznets,
davem, netdev, linux-kernel
In-Reply-To: <20190222182503.12160-1-haiyangz@linuxonhyperv.com>
On Fri, 22 Feb 2019 18:25:03 +0000
Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Incoming packets may have IP header checksum verified by the host.
> They may not have IP header checksum computed after coalescing.
> This patch re-compute the checksum when necessary, otherwise the
> packets may be dropped, because Linux network stack always checks it.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 256adbd044f5..cf4897043e83 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
> schedule_delayed_work(&ndev_ctx->dwork, 0);
> }
>
> +static void netvsc_comp_ipcsum(struct sk_buff *skb)
> +{
> + struct iphdr *iph = (struct iphdr *)skb->data;
Can you use iphdr(skb) here?
> +
> + iph->check = 0;
> + iph->check = ip_fast_csum(iph, iph->ihl);
> +}
> +
> static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> struct netvsc_channel *nvchan)
> {
> @@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> /* skb is already created with CHECKSUM_NONE */
> skb_checksum_none_assert(skb);
>
> - /*
> - * In Linux, the IP checksum is always checked.
> - * Do L4 checksum offload if enabled and present.
> + /* Incoming packets may have IP header checksum verified by the host.
> + * They may not have IP header checksum computed after coalescing.
> + * We compute it here if the flags are set, because on Linux, the IP
> + * checksum is always checked.
> + */
> + if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
> + csum_info->receive.ip_checksum_succeeded &&
> + skb->protocol == htons(ETH_P_IP))
> + netvsc_comp_ipcsum(skb);
Does this still handle for coalesced and non-coalesced packets
which are received with bad IP checksum? My concern is that you are
potentially correcting the checksum for a packet whose received checksum was bad.
> + /* Do L4 checksum offload if enabled and present.
> */
> if (csum_info && (net->features & NETIF_F_RXCSUM)) {
> if (csum_info->receive.tcp_checksum_succeeded ||
^ permalink raw reply
* Re: [GIT PULL v3] Hyper-V commits for 5.0-rc
From: Sasha Levin @ 2019-02-23 15:55 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hyperv, kys, haiyangz, sthemmin, linux-kernel
In-Reply-To: <20190223085715.GA22149@kroah.com>
On Sat, Feb 23, 2019 at 09:57:15AM +0100, Greg KH wrote:
>On Fri, Feb 22, 2019 at 10:12:25PM -0500, Sasha Levin wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA512
>>
>> The following changes since commit 8834f5600cf3c8db365e18a3d5cac2c2780c81e5:
>>
>> Linux 5.0-rc5 (2019-02-03 13:48:04 -0800)
>>
>> are available in the Git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
>>
>> for you to fetch changes up to b2946cd86f3c1b5b1262c0ec06068110c2328fe6:
>>
>> MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS (2019-02-22 21:46:37 -0500)
>>
>> - ----------------------------------------------------------------
>> Two fixes:
>>
>> 1. A fix for a race condition reading sysfs entries while a device is
>> being added, by Kimberly Brown.
>>
>> 2. Update the Hyper-V mailing list to a new one created on
>> vger.kernel.org, by Haiyang Zhang.
>>
>> - ----------------------------------------------------------------
>> Haiyang Zhang (1):
>> MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS
>>
>> Kimberly Brown (2):
>> Drivers: hv: vmbus: Change server monitor_pages index to 0
>> Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set
>
>I objected to this last patch when it was posted on the list. The sysfs
>file should just not be present if the functionality is not there, no
>need to add the "-EINVAL" logic to it instead.
>
>Having a sysfs file that says it can be read, and then rejecting that
>read with an error is NOT ok.
Hm, I'm sorry but I didn't see an objection on the thread
(https://lore.kernel.org/lkml/20190122020759.GA4054@ubu-Virtual-Machine/)
which is why it was sent in like this.
Could you please point me to it so we can get the patch fixed up?
--
Thanks,
Sasha
^ permalink raw reply
* Re: [GIT PULL v3] Hyper-V commits for 5.0-rc
From: Greg KH @ 2019-02-23 8:57 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, linux-hyperv, kys, haiyangz, sthemmin, linux-kernel
In-Reply-To: <20190223031226.AB912206A3@mail.kernel.org>
On Fri, Feb 22, 2019 at 10:12:25PM -0500, Sasha Levin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> The following changes since commit 8834f5600cf3c8db365e18a3d5cac2c2780c81e5:
>
> Linux 5.0-rc5 (2019-02-03 13:48:04 -0800)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
>
> for you to fetch changes up to b2946cd86f3c1b5b1262c0ec06068110c2328fe6:
>
> MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS (2019-02-22 21:46:37 -0500)
>
> - ----------------------------------------------------------------
> Two fixes:
>
> 1. A fix for a race condition reading sysfs entries while a device is
> being added, by Kimberly Brown.
>
> 2. Update the Hyper-V mailing list to a new one created on
> vger.kernel.org, by Haiyang Zhang.
>
> - ----------------------------------------------------------------
> Haiyang Zhang (1):
> MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS
>
> Kimberly Brown (2):
> Drivers: hv: vmbus: Change server monitor_pages index to 0
> Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set
I objected to this last patch when it was posted on the list. The sysfs
file should just not be present if the functionality is not there, no
need to add the "-EINVAL" logic to it instead.
Having a sysfs file that says it can be read, and then rejecting that
read with an error is NOT ok.
I'll cherry-pick the other two patches, but this one needs to be
reworked.
thanks,
greg k-h
^ permalink raw reply
* [GIT PULL v3] Hyper-V commits for 5.0-rc
From: Sasha Levin @ 2019-02-23 3:12 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-hyperv, kys, haiyangz, sthemmin, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
The following changes since commit 8834f5600cf3c8db365e18a3d5cac2c2780c81e5:
Linux 5.0-rc5 (2019-02-03 13:48:04 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed
for you to fetch changes up to b2946cd86f3c1b5b1262c0ec06068110c2328fe6:
MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS (2019-02-22 21:46:37 -0500)
- ----------------------------------------------------------------
Two fixes:
1. A fix for a race condition reading sysfs entries while a device is
being added, by Kimberly Brown.
2. Update the Hyper-V mailing list to a new one created on
vger.kernel.org, by Haiyang Zhang.
- ----------------------------------------------------------------
Haiyang Zhang (1):
MAINTAINERS: Change mailing list for Hyper-V CORE AND DRIVERS
Kimberly Brown (2):
Drivers: hv: vmbus: Change server monitor_pages index to 0
Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set
Documentation/ABI/stable/sysfs-bus-vmbus | 15 +++++++++---
MAINTAINERS | 2 +-
drivers/hv/vmbus_drv.c | 39 +++++++++++++++++++++++++++++++-
3 files changed, 51 insertions(+), 5 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEE4n5dijQDou9mhzu83qZv95d3LNwFAlxwucgACgkQ3qZv95d3
LNxkUg/9GmnREx3P9dvQECTeT/ynuL1kbA6lhbwolimrEjdBcEiPRRhD1UKOW5lb
lwBvp7Qj3+rnVBzI1+950VZoDCsmhCHS2DJ+Iy6OKPASYxCHXfSKfG0BWoid56ar
VTjG93gY9MmfUv/SkiAE/lYRdhGEM5GwojzPwKZ88+bV7gsgSHnFLeUgf/C5FXfG
PhvviKyV9S4MW5OwTcboNaHNSaS1Wf0EeOE3iHoXiE8Ylj3aJswrCgJWbIc9koQF
+KcugIIMnzTIhrKsxe9wmisAVAdaJo33suVOSgS50xnsVmIiJYtSgZ93/jH1GCXO
0rGuu1Qs/1dzzbZN5heQNm1KIlvpEaudG8pTQFssb3nmzyTPVFMqIb5pPungWpus
h4n/4DHg3pDgZXeTfygqogSkysWwfCoaiBymWfUYEbYJjg0zxgdrGbQvzowu5nOZ
mCBGxhrjJLDCMovkZKZggxutzIL+YTpJjPYmxZgNqnNdGHt9HR3syg142ZC3yXNT
ALZzQVXNk6cmCoTr9q/G3qbaS9Sv7BwMqC8YwGsqC5QbEP3WuKPBC7ifdXtA9nuo
fgAk2g3+vYlRK4kDUhl8EsTWMkH6HDNMe6Hzn4nyl01Qg36lyEwCoY1C32HFg8RR
Roi7nWz6IyQfJixYbkgVznhjHAuFZgIRS307a/n5vmwyYny1VZI=
=h1IS
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Haiyang Zhang @ 2019-02-22 18:25 UTC (permalink / raw)
To: sashal, linux-hyperv
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, netdev,
linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
Incoming packets may have IP header checksum verified by the host.
They may not have IP header checksum computed after coalescing.
This patch re-compute the checksum when necessary, otherwise the
packets may be dropped, because Linux network stack always checks it.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 256adbd044f5..cf4897043e83 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
schedule_delayed_work(&ndev_ctx->dwork, 0);
}
+static void netvsc_comp_ipcsum(struct sk_buff *skb)
+{
+ struct iphdr *iph = (struct iphdr *)skb->data;
+
+ iph->check = 0;
+ iph->check = ip_fast_csum(iph, iph->ihl);
+}
+
static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
struct netvsc_channel *nvchan)
{
@@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
/* skb is already created with CHECKSUM_NONE */
skb_checksum_none_assert(skb);
- /*
- * In Linux, the IP checksum is always checked.
- * Do L4 checksum offload if enabled and present.
+ /* Incoming packets may have IP header checksum verified by the host.
+ * They may not have IP header checksum computed after coalescing.
+ * We compute it here if the flags are set, because on Linux, the IP
+ * checksum is always checked.
+ */
+ if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
+ csum_info->receive.ip_checksum_succeeded &&
+ skb->protocol == htons(ETH_P_IP))
+ netvsc_comp_ipcsum(skb);
+
+ /* Do L4 checksum offload if enabled and present.
*/
if (csum_info && (net->features & NETIF_F_RXCSUM)) {
if (csum_info->receive.tcp_checksum_succeeded ||
--
2.19.1
^ permalink raw reply related
* RE: [PATCH V5 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
From: Michael Kelley @ 2019-02-22 15:58 UTC (permalink / raw)
To: Tianyu Lan
Cc: Tianyu Lan, joro@8bytes.org, mchehab+samsung@kernel.org,
davem@davemloft.net, gregkh@linuxfoundation.org,
nicolas.ferre@microchip.com, arnd@arndb.de,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
KY Srinivasan, vkuznets, alex.williamson@redhat.com,
sashal@kernel.org, dan.carpenter@oracle.com,
linux-hyperv@vger.kernel.org
In-Reply-To: <1550837545-10760-3-git-send-email-Tianyu.Lan@microsoft.com>
From: Tianyu.Lan@microsoft.com <Tianyu.Lan@microsoft.com> Sent: Friday, February 22, 2019 4:12 AM
>
> On the bare metal, enabling X2APIC mode requires interrupt remapping
> function which helps to deliver irq to cpu with 32-bit APIC ID.
> Hyper-V doesn't provide interrupt remapping function so far and Hyper-V
> MSI protocol already supports to deliver interrupt to the CPU whose
> virtual processor index is more than 255. IO-APIC interrupt still has
> 8-bit APIC ID limitation.
>
> This patch is to add Hyper-V stub IOMMU driver in order to enable
> X2APIC mode successfully in Hyper-V Linux guest. The driver returns X2APIC
> interrupt remapping capability when X2APIC mode is available. Otherwise,
> it creates a Hyper-V irq domain to limit IO-APIC interrupts' affinity
> and make sure cpus assigned with IO-APIC interrupt have 8-bit APIC ID.
>
> Define 24 IO-APIC remapping entries because Hyper-V only expose one
> single IO-APIC and one IO-APIC has 24 pins according IO-APIC spec(
> https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf).
>
> Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> ---
> Change since v4:
> - Fix the loop of scan cpu's APIC id
>
> Change since v3:
> - Make Hyper-V IOMMU as Hyper-V default driver
> - Fix hypervisor_is_type() input parameter
> - Check possible cpu numbers during scan 0~255 cpu's apic id.
>
> Change since v2:
> - Improve comment about why save IO-APIC entry in the irq chip data.
> - Some code improvement.
> - Improve statement in the IOMMU Kconfig.
>
> Change since v1:
> - Remove unused pr_fmt
> - Make ioapic_ir_domain as static variable
> - Remove unused variables cfg and entry in the hyperv_irq_remapping_alloc()
> - Fix comments
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V4 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
From: Tianyu Lan @ 2019-02-22 12:14 UTC (permalink / raw)
To: Michael Kelley, lantianyu1986@gmail.com
Cc: joro@8bytes.org, mchehab+samsung@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, nicolas.ferre@microchip.com,
arnd@arndb.de, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, KY Srinivasan, vkuznets,
alex.williamson@redhat.com, sashal@kernel.org,
dan.carpenter@oracle.com, linux-hyperv@vger.kernel.org
In-Reply-To: <DM5PR2101MB091885D1CD3761322191CAAFD77E0@DM5PR2101MB0918.namprd21.prod.outlook.com>
Hi Michael:
Thanks for your review.
-----Original Message-----
From: Michael Kelley <mikelley@microsoft.com>
Sent: Friday, February 22, 2019 1:28 AM
To: lantianyu1986@gmail.com
Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>; joro@8bytes.org; mchehab+samsung@kernel.org; davem@davemloft.net; gregkh@linuxfoundation.org; nicolas.ferre@microchip.com; arnd@arndb.de; linux-kernel@vger.kernel.org; iommu@lists.linux-foundation.org; KY Srinivasan <kys@microsoft.com>; vkuznets <vkuznets@redhat.com>; alex.williamson@redhat.com; sashal@kernel.org; dan.carpenter@oracle.com; linux-hyperv@vger.kernel.org
Subject: RE: [PATCH V4 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Monday, February 11, 2019 6:20 AM
> + /*
> + * Hyper-V doesn't provide irq remapping function for
> + * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
> + * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
> + * in the MADT table on Hyper-v are sorted monotonic increasingly.
> + * APIC ID reflects cpu topology. There maybe some APIC ID
> + * gaps when cpu number in a socket is not power of two. Prepare
> + * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
> + * into ioapic_max_cpumask if its APIC ID is less than 256.
> + */
> + for (i = min_t(unsigned int, num_possible_cpus(), 255); i >= 0; i--)
The above isn't quite right. For example, if num_possible_cpus() is 8, then the loop will be executed 9 times, for values 8 down through 0.
It should be executed for values 7 down through 0.
Yes, fix this in the V5. Thanks.
> + if (cpu_physical_id(i) < 256)
> + cpumask_set_cpu(i, &ioapic_max_cpumask);
> +
> + return 0;
> +}
Michael
^ permalink raw reply
* [PATCH v2 2/2] Drivers: hv: vmbus: Add a channel ring buffer mutex lock
From: Kimberly Brown @ 2019-02-22 3:47 UTC (permalink / raw)
To: Michael Kelley, Long Li, Sasha Levin, Stephen Hemminger,
Dexuan Cui
Cc: K. Y. Srinivasan, Haiyang Zhang, linux-hyperv, linux-kernel
In-Reply-To: <cover.1550806305.git.kimbrownkd@gmail.com>
The "_show" functions that access channel ring buffer data are
vulnerable to a race condition that can result in a NULL pointer
dereference. This problem was discussed here:
https://lkml.org/lkml/2018/10/18/779
To prevent this from occurring, add a new mutex lock,
"ring_buffer_mutex", to the vmbus_channel struct.
Acquire/release "ring_buffer_mutex" in the functions that can set the
ring buffer pointer to NULL: vmbus_free_ring() and __vmbus_open().
Acquire/release "ring_buffer_mutex" in the four channel-level "_show"
functions that access ring buffer data. Remove the "const" qualifier
from the "struct vmbus_channel *chan" parameter of the channel-level
"_show" functions so that "ring_buffer_mutex" can be acquired/released
in these functions.
Acquire/release "ring_buffer_mutex" in hv_ringbuffer_get_debuginfo().
Pass the channel pointer to hv_ringbuffer_get_debuginfo() so that
"ring_buffer_mutex" can be accessed in this function.
Signed-off-by: Kimberly Brown <kimbrownkd@gmail.com>
---
drivers/hv/channel.c | 5 ++
drivers/hv/channel_mgmt.c | 1 +
drivers/hv/ring_buffer.c | 11 +++-
drivers/hv/vmbus_drv.c | 111 ++++++++++++++++++++++++--------------
include/linux/hyperv.h | 10 +++-
5 files changed, 96 insertions(+), 42 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 23381c41d087..7770e97e4202 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -82,8 +82,10 @@ EXPORT_SYMBOL_GPL(vmbus_setevent);
/* vmbus_free_ring - drop mapping of ring buffer */
void vmbus_free_ring(struct vmbus_channel *channel)
{
+ mutex_lock(&channel->ring_buffer_mutex);
hv_ringbuffer_cleanup(&channel->outbound);
hv_ringbuffer_cleanup(&channel->inbound);
+ mutex_unlock(&channel->ring_buffer_mutex);
if (channel->ringbuffer_page) {
__free_pages(channel->ringbuffer_page,
@@ -241,8 +243,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
newchannel->ringbuffer_gpadlhandle = 0;
error_clean_ring:
+ mutex_lock(&newchannel->ring_buffer_mutex);
hv_ringbuffer_cleanup(&newchannel->outbound);
hv_ringbuffer_cleanup(&newchannel->inbound);
+ mutex_unlock(&newchannel->ring_buffer_mutex);
+
newchannel->state = CHANNEL_OPEN_STATE;
return err;
}
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 62703b354d6d..769873cddfe5 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -329,6 +329,7 @@ static struct vmbus_channel *alloc_channel(void)
spin_lock_init(&channel->lock);
init_completion(&channel->rescind_event);
+ mutex_init(&channel->ring_buffer_mutex);
INIT_LIST_HEAD(&channel->sc_list);
INIT_LIST_HEAD(&channel->percpu_list);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 9e8b31ccc142..35de60d2c1e8 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -167,13 +167,18 @@ hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
/* Get various debug metrics for the specified ring buffer. */
int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
- struct hv_ring_buffer_debug_info *debug_info)
+ struct hv_ring_buffer_debug_info *debug_info,
+ struct vmbus_channel *channel)
{
u32 bytes_avail_towrite;
u32 bytes_avail_toread;
- if (!ring_info->ring_buffer)
+ mutex_lock(&channel->ring_buffer_mutex);
+
+ if (!ring_info->ring_buffer) {
+ mutex_unlock(&channel->ring_buffer_mutex);
return -EINVAL;
+ }
hv_get_ringbuffer_availbytes(ring_info,
&bytes_avail_toread,
@@ -184,6 +189,8 @@ int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
debug_info->current_write_index = ring_info->ring_buffer->write_index;
debug_info->current_interrupt_mask
= ring_info->ring_buffer->interrupt_mask;
+ mutex_unlock(&channel->ring_buffer_mutex);
+
return 0;
}
EXPORT_SYMBOL_GPL(hv_ringbuffer_get_debuginfo);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b02bcf1a9380..1ff767795d0a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -345,9 +345,8 @@ static ssize_t out_intr_mask_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
-
ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,
- &outbound);
+ &outbound, hv_dev->channel);
if (ret < 0)
return ret;
@@ -366,7 +365,7 @@ static ssize_t out_read_index_show(struct device *dev,
return -ENODEV;
ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,
- &outbound);
+ &outbound, hv_dev->channel);
if (ret < 0)
return ret;
return sprintf(buf, "%d\n", outbound.current_read_index);
@@ -385,7 +384,7 @@ static ssize_t out_write_index_show(struct device *dev,
return -ENODEV;
ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,
- &outbound);
+ &outbound, hv_dev->channel);
if (ret < 0)
return ret;
return sprintf(buf, "%d\n", outbound.current_write_index);
@@ -404,7 +403,7 @@ static ssize_t out_read_bytes_avail_show(struct device *dev,
return -ENODEV;
ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,
- &outbound);
+ &outbound, hv_dev->channel);
if (ret < 0)
return ret;
return sprintf(buf, "%d\n", outbound.bytes_avail_toread);
@@ -423,7 +422,7 @@ static ssize_t out_write_bytes_avail_show(struct device *dev,
return -ENODEV;
ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound,
- &outbound);
+ &outbound, hv_dev->channel);
if (ret < 0)
return ret;
return sprintf(buf, "%d\n", outbound.bytes_avail_towrite);
@@ -440,7 +439,8 @@ static ssize_t in_intr_mask_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
- ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
+ ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound,
+ hv_dev->channel);
if (ret < 0)
return ret;
@@ -458,7 +458,8 @@ static ssize_t in_read_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
- ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
+ ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound,
+ hv_dev->channel);
if (ret < 0)
return ret;
@@ -476,7 +477,8 @@ static ssize_t in_write_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
- ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
+ ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound,
+ hv_dev->channel);
if (ret < 0)
return ret;
@@ -495,7 +497,8 @@ static ssize_t in_read_bytes_avail_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
- ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
+ ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound,
+ hv_dev->channel);
if (ret < 0)
return ret;
@@ -514,7 +517,8 @@ static ssize_t in_write_bytes_avail_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
- ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
+ ret = hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound,
+ hv_dev->channel);
if (ret < 0)
return ret;
@@ -1409,7 +1413,7 @@ static void vmbus_chan_release(struct kobject *kobj)
struct vmbus_chan_attribute {
struct attribute attr;
- ssize_t (*show)(const struct vmbus_channel *chan, char *buf);
+ ssize_t (*show)(struct vmbus_channel *chan, char *buf);
ssize_t (*store)(struct vmbus_channel *chan,
const char *buf, size_t count);
};
@@ -1428,7 +1432,7 @@ static ssize_t vmbus_chan_attr_show(struct kobject *kobj,
{
const struct vmbus_chan_attribute *attribute
= container_of(attr, struct vmbus_chan_attribute, attr);
- const struct vmbus_channel *chan
+ struct vmbus_channel *chan
= container_of(kobj, struct vmbus_channel, kobj);
if (!attribute->show)
@@ -1441,58 +1445,89 @@ static const struct sysfs_ops vmbus_chan_sysfs_ops = {
.show = vmbus_chan_attr_show,
};
-static ssize_t out_mask_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t out_mask_show(struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->outbound;
+ ssize_t ret;
+
+ mutex_lock(&channel->ring_buffer_mutex);
- if (!rbi->ring_buffer)
+ if (!rbi->ring_buffer) {
+ mutex_unlock(&channel->ring_buffer_mutex);
return -EINVAL;
+ }
- return sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
+ ret = sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
+ mutex_unlock(&channel->ring_buffer_mutex);
+
+ return ret;
}
static VMBUS_CHAN_ATTR_RO(out_mask);
-static ssize_t in_mask_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t in_mask_show(struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->inbound;
+ ssize_t ret;
- if (!rbi->ring_buffer)
+ mutex_lock(&channel->ring_buffer_mutex);
+
+ if (!rbi->ring_buffer) {
+ mutex_unlock(&channel->ring_buffer_mutex);
return -EINVAL;
+ }
+
+ ret = sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
+ mutex_unlock(&channel->ring_buffer_mutex);
- return sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
+ return ret;
}
static VMBUS_CHAN_ATTR_RO(in_mask);
-static ssize_t read_avail_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t read_avail_show(struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->inbound;
+ ssize_t ret;
+
+ mutex_lock(&channel->ring_buffer_mutex);
- if (!rbi->ring_buffer)
+ if (!rbi->ring_buffer) {
+ mutex_unlock(&channel->ring_buffer_mutex);
return -EINVAL;
+ }
+
+ ret = sprintf(buf, "%u\n", hv_get_bytes_to_read(rbi));
+ mutex_unlock(&channel->ring_buffer_mutex);
- return sprintf(buf, "%u\n", hv_get_bytes_to_read(rbi));
+ return ret;
}
static VMBUS_CHAN_ATTR_RO(read_avail);
-static ssize_t write_avail_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t write_avail_show(struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->outbound;
+ ssize_t ret;
- if (!rbi->ring_buffer)
+ mutex_lock(&channel->ring_buffer_mutex);
+
+ if (!rbi->ring_buffer) {
+ mutex_unlock(&channel->ring_buffer_mutex);
return -EINVAL;
+ }
- return sprintf(buf, "%u\n", hv_get_bytes_to_write(rbi));
+ ret = sprintf(buf, "%u\n", hv_get_bytes_to_write(rbi));
+ mutex_unlock(&channel->ring_buffer_mutex);
+
+ return ret;
}
static VMBUS_CHAN_ATTR_RO(write_avail);
-static ssize_t show_target_cpu(const struct vmbus_channel *channel, char *buf)
+static ssize_t show_target_cpu(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%u\n", channel->target_cpu);
}
static VMBUS_CHAN_ATTR(cpu, S_IRUGO, show_target_cpu, NULL);
-static ssize_t channel_pending_show(const struct vmbus_channel *channel,
- char *buf)
+static ssize_t channel_pending_show(struct vmbus_channel *channel, char *buf)
{
if (!channel->offermsg.monitor_allocated)
return -EINVAL;
@@ -1503,8 +1538,7 @@ static ssize_t channel_pending_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(pending, S_IRUGO, channel_pending_show, NULL);
-static ssize_t channel_latency_show(const struct vmbus_channel *channel,
- char *buf)
+static ssize_t channel_latency_show(struct vmbus_channel *channel, char *buf)
{
if (!channel->offermsg.monitor_allocated)
return -EINVAL;
@@ -1515,19 +1549,19 @@ static ssize_t channel_latency_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(latency, S_IRUGO, channel_latency_show, NULL);
-static ssize_t channel_interrupts_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t channel_interrupts_show(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%llu\n", channel->interrupts);
}
static VMBUS_CHAN_ATTR(interrupts, S_IRUGO, channel_interrupts_show, NULL);
-static ssize_t channel_events_show(const struct vmbus_channel *channel, char *buf)
+static ssize_t channel_events_show(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%llu\n", channel->sig_events);
}
static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
-static ssize_t channel_intr_in_full_show(const struct vmbus_channel *channel,
+static ssize_t channel_intr_in_full_show(struct vmbus_channel *channel,
char *buf)
{
return sprintf(buf, "%llu\n",
@@ -1535,7 +1569,7 @@ static ssize_t channel_intr_in_full_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(intr_in_full, 0444, channel_intr_in_full_show, NULL);
-static ssize_t channel_intr_out_empty_show(const struct vmbus_channel *channel,
+static ssize_t channel_intr_out_empty_show(struct vmbus_channel *channel,
char *buf)
{
return sprintf(buf, "%llu\n",
@@ -1543,7 +1577,7 @@ static ssize_t channel_intr_out_empty_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(intr_out_empty, 0444, channel_intr_out_empty_show, NULL);
-static ssize_t channel_out_full_first_show(const struct vmbus_channel *channel,
+static ssize_t channel_out_full_first_show(struct vmbus_channel *channel,
char *buf)
{
return sprintf(buf, "%llu\n",
@@ -1551,7 +1585,7 @@ static ssize_t channel_out_full_first_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(out_full_first, 0444, channel_out_full_first_show, NULL);
-static ssize_t channel_out_full_total_show(const struct vmbus_channel *channel,
+static ssize_t channel_out_full_total_show(struct vmbus_channel *channel,
char *buf)
{
return sprintf(buf, "%llu\n",
@@ -1559,7 +1593,7 @@ static ssize_t channel_out_full_total_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(out_full_total, 0444, channel_out_full_total_show, NULL);
-static ssize_t subchannel_monitor_id_show(const struct vmbus_channel *channel,
+static ssize_t subchannel_monitor_id_show(struct vmbus_channel *channel,
char *buf)
{
if (!channel->offermsg.monitor_allocated)
@@ -1569,8 +1603,7 @@ static ssize_t subchannel_monitor_id_show(const struct vmbus_channel *channel,
}
static VMBUS_CHAN_ATTR(monitor_id, S_IRUGO, subchannel_monitor_id_show, NULL);
-static ssize_t subchannel_id_show(const struct vmbus_channel *channel,
- char *buf)
+static ssize_t subchannel_id_show(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%u\n",
channel->offermsg.offer.sub_channel_index);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 64698ec8f2ac..6a6f79d7beba 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -934,6 +934,13 @@ struct vmbus_channel {
* full outbound ring buffer.
*/
u64 out_full_first;
+
+ /*
+ * The mutex lock that protects the channel's ring buffers. It's used to
+ * prevent the ring buffer pointers from being set to NULL while a
+ * function is accessing ring buffer data.
+ */
+ struct mutex ring_buffer_mutex;
};
static inline bool is_hvsock_channel(const struct vmbus_channel *c)
@@ -1207,7 +1214,8 @@ struct hv_ring_buffer_debug_info {
int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
- struct hv_ring_buffer_debug_info *debug_info);
+ struct hv_ring_buffer_debug_info *debug_info,
+ struct vmbus_channel *channel);
/* Vmbus interface */
#define vmbus_driver_register(driver) \
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/2] Drivers: hv: vmbus: Refactor chan->state if statement
From: Kimberly Brown @ 2019-02-22 3:47 UTC (permalink / raw)
To: Michael Kelley, Long Li, Sasha Levin, Stephen Hemminger,
Dexuan Cui
Cc: K. Y. Srinivasan, Haiyang Zhang, linux-hyperv, linux-kernel
In-Reply-To: <cover.1550806305.git.kimbrownkd@gmail.com>
The chan->state "if statement" was introduced in commit 6712cc9c2211
("vmbus: don't return values for uninitalized channels"). That commit
states that the purpose of the chan->state "if statement" is to prevent
returning garbage or causing a kernel OOPS when the channel ring buffer
is not initialized. The changes in this patch provide the same
protection.
Refactor the chan->state “if statement” in vmbus_chan_attr_show():
- Instead of checking the channel state in the "if statement", check
whether the channel ring buffer pointer is NULL. Checking the
ring buffer pointer makes this code consistent with
hv_ringbuffer_get_debuginfo().
- Move the "if statement" to the four "_show" functions that access a
channel ring buffer. Only four of the channel-level "_show" functions
access a ring buffer. The ring buffer pointer does not need to be
checked before calling the other "_show" functions, and moving the
ring buffer pointer "if statement" to the "_show" functions that
access a ring buffer makes the purpose of the "if statement" clear.
Signed-off-by: Kimberly Brown <kimbrownkd@gmail.com>
---
drivers/hv/vmbus_drv.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index f290401f97e5..b02bcf1a9380 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1434,9 +1434,6 @@ static ssize_t vmbus_chan_attr_show(struct kobject *kobj,
if (!attribute->show)
return -EIO;
- if (chan->state != CHANNEL_OPENED_STATE)
- return -EINVAL;
-
return attribute->show(chan, buf);
}
@@ -1448,6 +1445,9 @@ static ssize_t out_mask_show(const struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->outbound;
+ if (!rbi->ring_buffer)
+ return -EINVAL;
+
return sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
}
static VMBUS_CHAN_ATTR_RO(out_mask);
@@ -1456,6 +1456,9 @@ static ssize_t in_mask_show(const struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->inbound;
+ if (!rbi->ring_buffer)
+ return -EINVAL;
+
return sprintf(buf, "%u\n", rbi->ring_buffer->interrupt_mask);
}
static VMBUS_CHAN_ATTR_RO(in_mask);
@@ -1464,6 +1467,9 @@ static ssize_t read_avail_show(const struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->inbound;
+ if (!rbi->ring_buffer)
+ return -EINVAL;
+
return sprintf(buf, "%u\n", hv_get_bytes_to_read(rbi));
}
static VMBUS_CHAN_ATTR_RO(read_avail);
@@ -1472,6 +1478,9 @@ static ssize_t write_avail_show(const struct vmbus_channel *channel, char *buf)
{
const struct hv_ring_buffer_info *rbi = &channel->outbound;
+ if (!rbi->ring_buffer)
+ return -EINVAL;
+
return sprintf(buf, "%u\n", hv_get_bytes_to_write(rbi));
}
static VMBUS_CHAN_ATTR_RO(write_avail);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/2] Fix a race condition vulnerability in "_show" functions
From: Kimberly Brown @ 2019-02-22 3:46 UTC (permalink / raw)
To: Michael Kelley, Long Li, Sasha Levin, Stephen Hemminger,
Dexuan Cui
Cc: K. Y. Srinivasan, Haiyang Zhang, linux-hyperv, linux-kernel
In-Reply-To: <20190122020759.GA4054@ubu-Virtual-Machine>
This patchset fixes a race condition vulnerability in the "_show"
functions that access a channel ring buffer.
Changes in v2:
- In v1, I proposed using “vmbus_connection.channel_mutex” in the
“_show” functions to prevent the race condition. However, using this
mutex could result in a deadlock, so a new approach is needed.
- Patch 1 is new and consists of a code refactor.
- Patch 2 introduces a new mutex lock in the “vmbus_channel” struct,
and the new mutex is used to eliminate the race condition.
Kimberly Brown (2):
Drivers: hv: vmbus: Refactor chan->state if statement
Drivers: hv: vmbus: Add a channel ring buffer mutex lock
drivers/hv/channel.c | 5 ++
drivers/hv/channel_mgmt.c | 1 +
drivers/hv/ring_buffer.c | 11 +++-
drivers/hv/vmbus_drv.c | 118 ++++++++++++++++++++++++++------------
include/linux/hyperv.h | 10 +++-
5 files changed, 104 insertions(+), 41 deletions(-)
--
2.17.1
^ permalink raw reply
* RE: [PATCH V4 3/3] MAINTAINERS: Add Hyper-V IOMMU driver into Hyper-V CORE AND DRIVERS scope
From: Michael Kelley @ 2019-02-21 17:28 UTC (permalink / raw)
To: lantianyu1986@gmail.com
Cc: Tianyu Lan, mchehab+samsung@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, nicolas.ferre@microchip.com,
arnd@arndb.de, linux-kernel@vger.kernel.org, KY Srinivasan,
vkuznets, alex.williamson@redhat.com, joro@8bytes.org,
sashal@kernel.org, dan.carpenter@oracle.com,
linux-hyperv@vger.kernel.org
In-Reply-To: <1549894824-26623-4-git-send-email-Tianyu.Lan@microsoft.com>
From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Monday, February 11, 2019 6:20 AM
>
> This patch is to add Hyper-V IOMMU driver file into Hyper-V CORE and
> DRIVERS scope.
>
> Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> ---
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH V4 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
From: Michael Kelley @ 2019-02-21 17:27 UTC (permalink / raw)
To: lantianyu1986@gmail.com
Cc: Tianyu Lan, joro@8bytes.org, mchehab+samsung@kernel.org,
davem@davemloft.net, gregkh@linuxfoundation.org,
nicolas.ferre@microchip.com, arnd@arndb.de,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
KY Srinivasan, vkuznets, alex.williamson@redhat.com,
sashal@kernel.org, dan.carpenter@oracle.com,
linux-hyperv@vger.kernel.org
In-Reply-To: <1549894824-26623-3-git-send-email-Tianyu.Lan@microsoft.com>
From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Monday, February 11, 2019 6:20 AM
> + /*
> + * Hyper-V doesn't provide irq remapping function for
> + * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
> + * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
> + * in the MADT table on Hyper-v are sorted monotonic increasingly.
> + * APIC ID reflects cpu topology. There maybe some APIC ID
> + * gaps when cpu number in a socket is not power of two. Prepare
> + * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
> + * into ioapic_max_cpumask if its APIC ID is less than 256.
> + */
> + for (i = min_t(unsigned int, num_possible_cpus(), 255); i >= 0; i--)
The above isn't quite right. For example, if num_possible_cpus() is 8,
then the loop will be executed 9 times, for values 8 down through 0.
It should be executed for values 7 down through 0.
> + if (cpu_physical_id(i) < 256)
> + cpumask_set_cpu(i, &ioapic_max_cpumask);
> +
> + return 0;
> +}
Michael
^ permalink raw reply
* RE: [PATCH V4 1/3] x86/Hyper-V: Set x2apic destination mode to physical when x2apic is available
From: Michael Kelley @ 2019-02-21 17:00 UTC (permalink / raw)
To: lantianyu1986@gmail.com
Cc: Tianyu Lan, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, hpa@zytor.com, x86@kernel.org, joro@8bytes.org,
mchehab+samsung@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, nicolas.ferre@microchip.com,
arnd@arndb.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, iommu@lists.linux-foundation.org,
vkuznets, alex.williamson@redhat.com, dan.carpenter@oracle.com,
linux-hyperv@vger.kernel.org
In-Reply-To: <1549894824-26623-2-git-send-email-Tianyu.Lan@microsoft.com>
From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Monday, February 11, 2019 6:20 AM
>
> Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
> set x2apic destination mode to physcial mode when x2apic is available
> and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs have
> 8-bit APIC id.
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> ---
> Change since v2:
> - Fix compile error due to x2apic_phys
> - Fix comment indent
> Change since v1:
> - Remove redundant extern for x2apic_phys
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* Testing delivery to lore.kernel.org
From: Konstantin Ryabitsev @ 2019-02-21 16:16 UTC (permalink / raw)
To: linux-hyperv
[-- Attachment #1: Type: text/plain, Size: 142 bytes --]
This is a test for delivering to lore.kernel.org/linux-hyperv. It is a
required step for initializing empty archives. Please ignore.
-K
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox