* [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