* Re: [Patch v4 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Lorenzo Pieralisi @ 2020-02-25 10:36 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Andrew Murray, Bjorn Helgaas, linux-hyperv, linux-pci,
linux-kernel, Long Li
In-Reply-To: <1578946101-74036-2-git-send-email-longli@linuxonhyperv.com>
On Mon, Jan 13, 2020 at 12:08:21PM -0800, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the
> bus. The vNUMA node tells which guest NUMA node this device is on based
> on guest VM configuration topology and physical device inforamtion.
s/inforamtion/information
Please rebase this series on top of my pci/hv branch, it does
not apply, I will merge it then.
Thanks,
Lorenzo
> Add code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> Changes
> v2: Changed some spaces to tabs, added put_pcichild() after get_pcichild_wslot(), renamed pci_assign_numa_node() to hv_pci_assign_numa_node()
> v4: Fixed spelling
>
> drivers/pci/controller/pci-hyperv.c | 109 ++++++++++++++++++++++++++++
> 1 file changed, 109 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 3b3e1967cf08..147358fae8a2 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -63,6 +63,7 @@
> enum pci_protocol_version_t {
> PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /* Win10 */
> PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
> + PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* Vibranium */
> };
>
> #define CPU_AFFINITY_ALL -1ULL
> @@ -72,6 +73,7 @@ enum pci_protocol_version_t {
> * first.
> */
> static enum pci_protocol_version_t pci_protocol_versions[] = {
> + PCI_PROTOCOL_VERSION_1_3,
> PCI_PROTOCOL_VERSION_1_2,
> PCI_PROTOCOL_VERSION_1_1,
> };
> @@ -124,6 +126,7 @@ enum pci_message_type {
> PCI_RESOURCES_ASSIGNED2 = PCI_MESSAGE_BASE + 0x16,
> PCI_CREATE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x17,
> PCI_DELETE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x18, /* unused */
> + PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
> PCI_MESSAGE_MAXIMUM
> };
>
> @@ -169,6 +172,26 @@ struct pci_function_description {
> u32 ser; /* serial number */
> } __packed;
>
> +enum pci_device_description_flags {
> + HV_PCI_DEVICE_FLAG_NONE = 0x0,
> + HV_PCI_DEVICE_FLAG_NUMA_AFFINITY = 0x1,
> +};
> +
> +struct pci_function_description2 {
> + u16 v_id; /* vendor ID */
> + u16 d_id; /* device ID */
> + u8 rev;
> + u8 prog_intf;
> + u8 subclass;
> + u8 base_class;
> + u32 subsystem_id;
> + union win_slot_encoding win_slot;
> + u32 ser; /* serial number */
> + u32 flags;
> + u16 virtual_numa_node;
> + u16 reserved;
> +} __packed;
> +
> /**
> * struct hv_msi_desc
> * @vector: IDT entry
> @@ -304,6 +327,12 @@ struct pci_bus_relations {
> struct pci_function_description func[0];
> } __packed;
>
> +struct pci_bus_relations2 {
> + struct pci_incoming_message incoming;
> + u32 device_count;
> + struct pci_function_description2 func[0];
> +} __packed;
> +
> struct pci_q_res_req_response {
> struct vmpacket_descriptor hdr;
> s32 status; /* negative values are failures */
> @@ -1417,6 +1446,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> break;
>
> case PCI_PROTOCOL_VERSION_1_2:
> + case PCI_PROTOCOL_VERSION_1_3:
> size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
> dest,
> hpdev->desc.win_slot.slot,
> @@ -1798,6 +1828,27 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
> }
> }
>
> +/*
> + * Set NUMA node for the devices on the bus
> + */
> +static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
> +{
> + struct pci_dev *dev;
> + struct pci_bus *bus = hbus->pci_bus;
> + struct hv_pci_dev *hv_dev;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
> + if (!hv_dev)
> + continue;
> +
> + if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
> + set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
> +
> + put_pcichild(hv_dev);
> + }
> +}
> +
> /**
> * create_root_hv_pci_bus() - Expose a new root PCI bus
> * @hbus: Root PCI bus, as understood by this driver
> @@ -1820,6 +1871,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
>
> pci_lock_rescan_remove();
> pci_scan_child_bus(hbus->pci_bus);
> + hv_pci_assign_numa_node(hbus);
> pci_bus_assign_resources(hbus->pci_bus);
> hv_pci_assign_slots(hbus);
> pci_bus_add_devices(hbus->pci_bus);
> @@ -2088,6 +2140,7 @@ static void pci_devices_present_work(struct work_struct *work)
> */
> pci_lock_rescan_remove();
> pci_scan_child_bus(hbus->pci_bus);
> + hv_pci_assign_numa_node(hbus);
> hv_pci_assign_slots(hbus);
> pci_unlock_rescan_remove();
> break;
> @@ -2184,6 +2237,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> kfree(dr);
> }
>
> +/**
> + * hv_pci_devices_present2() - Handle list of new children
> + * @hbus: Root PCI bus, as understood by this driver
> + * @relations2: Packet from host listing children
> + *
> + * This function is the v2 version of hv_pci_devices_present()
> + */
> +static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
> + struct pci_bus_relations2 *relations)
> +{
> + struct hv_dr_state *dr;
> + int i;
> +
> + dr = kzalloc(offsetof(struct hv_dr_state, func) +
> + (sizeof(struct hv_pcidev_description) *
> + (relations->device_count)), GFP_NOWAIT);
> +
> + if (!dr)
> + return;
> +
> + dr->device_count = relations->device_count;
> + for (i = 0; i < dr->device_count; i++) {
> + dr->func[i].v_id = relations->func[i].v_id;
> + dr->func[i].d_id = relations->func[i].d_id;
> + dr->func[i].rev = relations->func[i].rev;
> + dr->func[i].prog_intf = relations->func[i].prog_intf;
> + dr->func[i].subclass = relations->func[i].subclass;
> + dr->func[i].base_class = relations->func[i].base_class;
> + dr->func[i].subsystem_id = relations->func[i].subsystem_id;
> + dr->func[i].win_slot = relations->func[i].win_slot;
> + dr->func[i].ser = relations->func[i].ser;
> + dr->func[i].flags = relations->func[i].flags;
> + dr->func[i].virtual_numa_node =
> + relations->func[i].virtual_numa_node;
> + }
> +
> + if (hv_pci_start_relations_work(hbus, dr))
> + kfree(dr);
> +}
> +
> /**
> * hv_eject_device_work() - Asynchronously handles ejection
> * @work: Work struct embedded in internal device struct
> @@ -2289,6 +2382,7 @@ static void hv_pci_onchannelcallback(void *context)
> struct pci_response *response;
> struct pci_incoming_message *new_message;
> struct pci_bus_relations *bus_rel;
> + struct pci_bus_relations2 *bus_rel2;
> struct pci_dev_inval_block *inval;
> struct pci_dev_incoming *dev_message;
> struct hv_pci_dev *hpdev;
> @@ -2356,6 +2450,21 @@ static void hv_pci_onchannelcallback(void *context)
> hv_pci_devices_present(hbus, bus_rel);
> break;
>
> + case PCI_BUS_RELATIONS2:
> +
> + bus_rel2 = (struct pci_bus_relations2 *)buffer;
> + if (bytes_recvd <
> + offsetof(struct pci_bus_relations2, func) +
> + (sizeof(struct pci_function_description2) *
> + (bus_rel2->device_count))) {
> + dev_err(&hbus->hdev->device,
> + "bus relations v2 too small\n");
> + break;
> + }
> +
> + hv_pci_devices_present2(hbus, bus_rel2);
> + break;
> +
> case PCI_EJECT:
>
> dev_message = (struct pci_dev_incoming *)buffer;
> --
> 2.17.1
>
^ permalink raw reply
* RE: [Patch v4 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Long Li @ 2020-02-25 21:28 UTC (permalink / raw)
To: Lorenzo Pieralisi, longli@linuxonhyperv.com
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Andrew Murray, Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200225103607.GB4029@e121166-lin.cambridge.arm.com>
>Subject: Re: [Patch v4 2/2] PCI: hv: Add support for protocol 1.3 and support
>PCI_BUS_RELATIONS2
>
>On Mon, Jan 13, 2020 at 12:08:21PM -0800, longli@linuxonhyperv.com wrote:
>> From: Long Li <longli@microsoft.com>
>>
>> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
>> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices
>on
>> the bus. The vNUMA node tells which guest NUMA node this device is on
>> based on guest VM configuration topology and physical device inforamtion.
>
>s/inforamtion/information
>
>Please rebase this series on top of my pci/hv branch, it does not apply, I will
>merge it then.
>
>Thanks,
>Lorenzo
I will send v5.
Thanks,
Long
>
>> Add code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
>>
>> Signed-off-by: Long Li <longli@microsoft.com>
>> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>> ---
>> Changes
>> v2: Changed some spaces to tabs, added put_pcichild() after
>> get_pcichild_wslot(), renamed pci_assign_numa_node() to
>> hv_pci_assign_numa_node()
>> v4: Fixed spelling
>>
>> drivers/pci/controller/pci-hyperv.c | 109
>> ++++++++++++++++++++++++++++
>> 1 file changed, 109 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-hyperv.c
>> b/drivers/pci/controller/pci-hyperv.c
>> index 3b3e1967cf08..147358fae8a2 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -63,6 +63,7 @@
>> enum pci_protocol_version_t {
>> PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /*
>Win10 */
>> PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1
>*/
>> + PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /*
>Vibranium */
>> };
>>
>> #define CPU_AFFINITY_ALL -1ULL
>> @@ -72,6 +73,7 @@ enum pci_protocol_version_t {
>> * first.
>> */
>> static enum pci_protocol_version_t pci_protocol_versions[] = {
>> + PCI_PROTOCOL_VERSION_1_3,
>> PCI_PROTOCOL_VERSION_1_2,
>> PCI_PROTOCOL_VERSION_1_1,
>> };
>> @@ -124,6 +126,7 @@ enum pci_message_type {
>> PCI_RESOURCES_ASSIGNED2 = PCI_MESSAGE_BASE + 0x16,
>> PCI_CREATE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x17,
>> PCI_DELETE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x18,
>/* unused */
>> + PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
>> PCI_MESSAGE_MAXIMUM
>> };
>>
>> @@ -169,6 +172,26 @@ struct pci_function_description {
>> u32 ser; /* serial number */
>> } __packed;
>>
>> +enum pci_device_description_flags {
>> + HV_PCI_DEVICE_FLAG_NONE = 0x0,
>> + HV_PCI_DEVICE_FLAG_NUMA_AFFINITY = 0x1,
>> +};
>> +
>> +struct pci_function_description2 {
>> + u16 v_id; /* vendor ID */
>> + u16 d_id; /* device ID */
>> + u8 rev;
>> + u8 prog_intf;
>> + u8 subclass;
>> + u8 base_class;
>> + u32 subsystem_id;
>> + union win_slot_encoding win_slot;
>> + u32 ser; /* serial number */
>> + u32 flags;
>> + u16 virtual_numa_node;
>> + u16 reserved;
>> +} __packed;
>> +
>> /**
>> * struct hv_msi_desc
>> * @vector: IDT entry
>> @@ -304,6 +327,12 @@ struct pci_bus_relations {
>> struct pci_function_description func[0]; } __packed;
>>
>> +struct pci_bus_relations2 {
>> + struct pci_incoming_message incoming;
>> + u32 device_count;
>> + struct pci_function_description2 func[0]; } __packed;
>> +
>> struct pci_q_res_req_response {
>> struct vmpacket_descriptor hdr;
>> s32 status; /* negative values are failures */
>> @@ -1417,6 +1446,7 @@ static void hv_compose_msi_msg(struct irq_data
>*data, struct msi_msg *msg)
>> break;
>>
>> case PCI_PROTOCOL_VERSION_1_2:
>> + case PCI_PROTOCOL_VERSION_1_3:
>> size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
>> dest,
>> hpdev->desc.win_slot.slot,
>> @@ -1798,6 +1828,27 @@ static void hv_pci_remove_slots(struct
>hv_pcibus_device *hbus)
>> }
>> }
>>
>> +/*
>> + * Set NUMA node for the devices on the bus */ static void
>> +hv_pci_assign_numa_node(struct hv_pcibus_device *hbus) {
>> + struct pci_dev *dev;
>> + struct pci_bus *bus = hbus->pci_bus;
>> + struct hv_pci_dev *hv_dev;
>> +
>> + list_for_each_entry(dev, &bus->devices, bus_list) {
>> + hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev-
>>devfn));
>> + if (!hv_dev)
>> + continue;
>> +
>> + if (hv_dev->desc.flags &
>HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
>> + set_dev_node(&dev->dev, hv_dev-
>>desc.virtual_numa_node);
>> +
>> + put_pcichild(hv_dev);
>> + }
>> +}
>> +
>> /**
>> * create_root_hv_pci_bus() - Expose a new root PCI bus
>> * @hbus: Root PCI bus, as understood by this driver
>> @@ -1820,6 +1871,7 @@ static int create_root_hv_pci_bus(struct
>> hv_pcibus_device *hbus)
>>
>> pci_lock_rescan_remove();
>> pci_scan_child_bus(hbus->pci_bus);
>> + hv_pci_assign_numa_node(hbus);
>> pci_bus_assign_resources(hbus->pci_bus);
>> hv_pci_assign_slots(hbus);
>> pci_bus_add_devices(hbus->pci_bus);
>> @@ -2088,6 +2140,7 @@ static void pci_devices_present_work(struct
>work_struct *work)
>> */
>> pci_lock_rescan_remove();
>> pci_scan_child_bus(hbus->pci_bus);
>> + hv_pci_assign_numa_node(hbus);
>> hv_pci_assign_slots(hbus);
>> pci_unlock_rescan_remove();
>> break;
>> @@ -2184,6 +2237,46 @@ static void hv_pci_devices_present(struct
>hv_pcibus_device *hbus,
>> kfree(dr);
>> }
>>
>> +/**
>> + * hv_pci_devices_present2() - Handle list of new children
>> + * @hbus: Root PCI bus, as understood by this driver
>> + * @relations2: Packet from host listing children
>> + *
>> + * This function is the v2 version of hv_pci_devices_present() */
>> +static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
>> + struct pci_bus_relations2 *relations) {
>> + struct hv_dr_state *dr;
>> + int i;
>> +
>> + dr = kzalloc(offsetof(struct hv_dr_state, func) +
>> + (sizeof(struct hv_pcidev_description) *
>> + (relations->device_count)), GFP_NOWAIT);
>> +
>> + if (!dr)
>> + return;
>> +
>> + dr->device_count = relations->device_count;
>> + for (i = 0; i < dr->device_count; i++) {
>> + dr->func[i].v_id = relations->func[i].v_id;
>> + dr->func[i].d_id = relations->func[i].d_id;
>> + dr->func[i].rev = relations->func[i].rev;
>> + dr->func[i].prog_intf = relations->func[i].prog_intf;
>> + dr->func[i].subclass = relations->func[i].subclass;
>> + dr->func[i].base_class = relations->func[i].base_class;
>> + dr->func[i].subsystem_id = relations->func[i].subsystem_id;
>> + dr->func[i].win_slot = relations->func[i].win_slot;
>> + dr->func[i].ser = relations->func[i].ser;
>> + dr->func[i].flags = relations->func[i].flags;
>> + dr->func[i].virtual_numa_node =
>> + relations->func[i].virtual_numa_node;
>> + }
>> +
>> + if (hv_pci_start_relations_work(hbus, dr))
>> + kfree(dr);
>> +}
>> +
>> /**
>> * hv_eject_device_work() - Asynchronously handles ejection
>> * @work: Work struct embedded in internal device struct
>> @@ -2289,6 +2382,7 @@ static void hv_pci_onchannelcallback(void
>*context)
>> struct pci_response *response;
>> struct pci_incoming_message *new_message;
>> struct pci_bus_relations *bus_rel;
>> + struct pci_bus_relations2 *bus_rel2;
>> struct pci_dev_inval_block *inval;
>> struct pci_dev_incoming *dev_message;
>> struct hv_pci_dev *hpdev;
>> @@ -2356,6 +2450,21 @@ static void hv_pci_onchannelcallback(void
>*context)
>> hv_pci_devices_present(hbus, bus_rel);
>> break;
>>
>> + case PCI_BUS_RELATIONS2:
>> +
>> + bus_rel2 = (struct pci_bus_relations2 *)buffer;
>> + if (bytes_recvd <
>> + offsetof(struct pci_bus_relations2, func) +
>> + (sizeof(struct pci_function_description2) *
>> + (bus_rel2->device_count))) {
>> + dev_err(&hbus->hdev->device,
>> + "bus relations v2 too small\n");
>> + break;
>> + }
>> +
>> + hv_pci_devices_present2(hbus, bus_rel2);
>> + break;
>> +
>> case PCI_EJECT:
>>
>> dev_message = (struct pci_dev_incoming
>*)buffer;
>> --
>> 2.17.1
>>
^ permalink raw reply
* [Patch v5 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message
From: longli @ 2020-02-26 5:06 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas, linux-hyperv,
linux-pci, linux-kernel
Cc: Long Li
From: Long Li <longli@microsoft.com>
hv_dr_state is used to find present PCI devices on the bus. The structure
reuses struct pci_function_description from VSP message to describe a
device.
To prepare support for pci_function_description v2, decouple this
dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
There is no functionality change.
Signed-off-by: Long Li <longli@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
Changes
v2: Changed some spaces to tabs, changed failure code to -ENOMEM
v3: Revised comment for function hv_pci_devices_present(), reformatted patch title
v4: Fixed spelling
v5: Rebased to current tree
drivers/pci/controller/pci-hyperv.c | 100 +++++++++++++++++++++++++-----------
1 file changed, 70 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 15011a3..dea197f 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -505,10 +505,24 @@ struct hv_dr_work {
struct hv_pcibus_device *bus;
};
+struct hv_pcidev_description {
+ u16 v_id; /* vendor ID */
+ u16 d_id; /* device ID */
+ u8 rev;
+ u8 prog_intf;
+ u8 subclass;
+ u8 base_class;
+ u32 subsystem_id;
+ union win_slot_encoding win_slot;
+ u32 ser; /* serial number */
+ u32 flags;
+ u16 virtual_numa_node;
+};
+
struct hv_dr_state {
struct list_head list_entry;
u32 device_count;
- struct pci_function_description func[0];
+ struct hv_pcidev_description func[0];
};
enum hv_pcichild_state {
@@ -525,7 +539,7 @@ struct hv_pci_dev {
refcount_t refs;
enum hv_pcichild_state state;
struct pci_slot *pci_slot;
- struct pci_function_description desc;
+ struct hv_pcidev_description desc;
bool reported_missing;
struct hv_pcibus_device *hbus;
struct work_struct wrk;
@@ -1877,7 +1891,7 @@ static void q_resource_requirements(void *context, struct pci_response *resp,
* Return: Pointer to the new tracking struct
*/
static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
- struct pci_function_description *desc)
+ struct hv_pcidev_description *desc)
{
struct hv_pci_dev *hpdev;
struct pci_child_message *res_req;
@@ -1988,7 +2002,7 @@ static void pci_devices_present_work(struct work_struct *work)
{
u32 child_no;
bool found;
- struct pci_function_description *new_desc;
+ struct hv_pcidev_description *new_desc;
struct hv_pci_dev *hpdev;
struct hv_pcibus_device *hbus;
struct list_head removed;
@@ -2107,17 +2121,15 @@ static void pci_devices_present_work(struct work_struct *work)
}
/**
- * hv_pci_devices_present() - Handles list of new children
+ * hv_pci_start_relations_work() - Queue work to start device discovery
* @hbus: Root PCI bus, as understood by this driver
- * @relations: Packet from host listing children
+ * @dr: The list of children returned from host
*
- * This function is invoked whenever a new list of devices for
- * this bus appears.
+ * Return: 0 on success, -errno on failure
*/
-static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
- struct pci_bus_relations *relations)
+static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
+ struct hv_dr_state *dr)
{
- struct hv_dr_state *dr;
struct hv_dr_work *dr_wrk;
unsigned long flags;
bool pending_dr;
@@ -2125,29 +2137,15 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
if (hbus->state == hv_pcibus_removing) {
dev_info(&hbus->hdev->device,
"PCI VMBus BUS_RELATIONS: ignored\n");
- return;
+ return -ENOENT;
}
dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
if (!dr_wrk)
- return;
-
- dr = kzalloc(offsetof(struct hv_dr_state, func) +
- (sizeof(struct pci_function_description) *
- (relations->device_count)), GFP_NOWAIT);
- if (!dr) {
- kfree(dr_wrk);
- return;
- }
+ return -ENOMEM;
INIT_WORK(&dr_wrk->wrk, pci_devices_present_work);
dr_wrk->bus = hbus;
- dr->device_count = relations->device_count;
- if (dr->device_count != 0) {
- memcpy(dr->func, relations->func,
- sizeof(struct pci_function_description) *
- dr->device_count);
- }
spin_lock_irqsave(&hbus->device_list_lock, flags);
/*
@@ -2165,6 +2163,47 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
get_hvpcibus(hbus);
queue_work(hbus->wq, &dr_wrk->wrk);
}
+
+ return 0;
+}
+
+/**
+ * hv_pci_devices_present() - Handle list of new children
+ * @hbus: Root PCI bus, as understood by this driver
+ * @relations: Packet from host listing children
+ *
+ * Process a new list of devices on the bus. The list of devices is
+ * discovered by VSP and sent to us via VSP message PCI_BUS_RELATIONS,
+ * whenever a new list of devices for this bus appears.
+ */
+static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
+ struct pci_bus_relations *relations)
+{
+ struct hv_dr_state *dr;
+ int i;
+
+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
+ (sizeof(struct hv_pcidev_description) *
+ (relations->device_count)), GFP_NOWAIT);
+
+ if (!dr)
+ return;
+
+ dr->device_count = relations->device_count;
+ for (i = 0; i < dr->device_count; i++) {
+ dr->func[i].v_id = relations->func[i].v_id;
+ dr->func[i].d_id = relations->func[i].d_id;
+ dr->func[i].rev = relations->func[i].rev;
+ dr->func[i].prog_intf = relations->func[i].prog_intf;
+ dr->func[i].subclass = relations->func[i].subclass;
+ dr->func[i].base_class = relations->func[i].base_class;
+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
+ dr->func[i].win_slot = relations->func[i].win_slot;
+ dr->func[i].ser = relations->func[i].ser;
+ }
+
+ if (hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
}
/**
@@ -3069,7 +3108,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
struct pci_packet teardown_packet;
u8 buffer[sizeof(struct pci_message)];
} pkt;
- struct pci_bus_relations relations;
+ struct hv_dr_state *dr;
struct hv_pci_compl comp_pkt;
int ret;
@@ -3082,8 +3121,9 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
if (!hibernating) {
/* Delete any children which might still exist. */
- memset(&relations, 0, sizeof(relations));
- hv_pci_devices_present(hbus, &relations);
+ dr = kzalloc(sizeof(*dr), GFP_KERNEL);
+ if (dr && hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
}
ret = hv_send_resources_released(hdev);
--
1.8.3.1
^ permalink raw reply related
* [Patch v5 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: longli @ 2020-02-26 5:06 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas, linux-hyperv,
linux-pci, linux-kernel
Cc: Long Li
In-Reply-To: <1582693568-64759-1-git-send-email-longli@linuxonhyperv.com>
From: Long Li <longli@microsoft.com>
Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the
bus. The vNUMA node tells which guest NUMA node this device is on based
on guest VM configuration topology and physical device information.
Add code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
Signed-off-by: Long Li <longli@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
Changes
v2: Changed some spaces to tabs, added put_pcichild() after get_pcichild_wslot(), renamed pci_assign_numa_node() to hv_pci_assign_numa_node()
v4: Fixed spelling
v5: Fixed spelling, rebased to current tree
drivers/pci/controller/pci-hyperv.c | 109 ++++++++++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index dea197f..98d3776 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -63,6 +63,7 @@
enum pci_protocol_version_t {
PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1), /* Win10 */
PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
+ PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* Vibranium */
};
#define CPU_AFFINITY_ALL -1ULL
@@ -72,6 +73,7 @@ enum pci_protocol_version_t {
* first.
*/
static enum pci_protocol_version_t pci_protocol_versions[] = {
+ PCI_PROTOCOL_VERSION_1_3,
PCI_PROTOCOL_VERSION_1_2,
PCI_PROTOCOL_VERSION_1_1,
};
@@ -119,6 +121,7 @@ enum pci_message_type {
PCI_RESOURCES_ASSIGNED2 = PCI_MESSAGE_BASE + 0x16,
PCI_CREATE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x17,
PCI_DELETE_INTERRUPT_MESSAGE2 = PCI_MESSAGE_BASE + 0x18, /* unused */
+ PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
PCI_MESSAGE_MAXIMUM
};
@@ -164,6 +167,26 @@ struct pci_function_description {
u32 ser; /* serial number */
} __packed;
+enum pci_device_description_flags {
+ HV_PCI_DEVICE_FLAG_NONE = 0x0,
+ HV_PCI_DEVICE_FLAG_NUMA_AFFINITY = 0x1,
+};
+
+struct pci_function_description2 {
+ u16 v_id; /* vendor ID */
+ u16 d_id; /* device ID */
+ u8 rev;
+ u8 prog_intf;
+ u8 subclass;
+ u8 base_class;
+ u32 subsystem_id;
+ union win_slot_encoding win_slot;
+ u32 ser; /* serial number */
+ u32 flags;
+ u16 virtual_numa_node;
+ u16 reserved;
+} __packed;
+
/**
* struct hv_msi_desc
* @vector: IDT entry
@@ -299,6 +322,12 @@ struct pci_bus_relations {
struct pci_function_description func[0];
} __packed;
+struct pci_bus_relations2 {
+ struct pci_incoming_message incoming;
+ u32 device_count;
+ struct pci_function_description2 func[0];
+} __packed;
+
struct pci_q_res_req_response {
struct vmpacket_descriptor hdr;
s32 status; /* negative values are failures */
@@ -1415,6 +1444,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
break;
case PCI_PROTOCOL_VERSION_1_2:
+ case PCI_PROTOCOL_VERSION_1_3:
size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
dest,
hpdev->desc.win_slot.slot,
@@ -1813,6 +1843,27 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
}
}
+/*
+ * Set NUMA node for the devices on the bus
+ */
+static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
+{
+ struct pci_dev *dev;
+ struct pci_bus *bus = hbus->pci_bus;
+ struct hv_pci_dev *hv_dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
+ if (!hv_dev)
+ continue;
+
+ if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
+ set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
+
+ put_pcichild(hv_dev);
+ }
+}
+
/**
* create_root_hv_pci_bus() - Expose a new root PCI bus
* @hbus: Root PCI bus, as understood by this driver
@@ -1835,6 +1886,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ hv_pci_assign_numa_node(hbus);
pci_bus_assign_resources(hbus->pci_bus);
hv_pci_assign_slots(hbus);
pci_bus_add_devices(hbus->pci_bus);
@@ -2103,6 +2155,7 @@ static void pci_devices_present_work(struct work_struct *work)
*/
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
+ hv_pci_assign_numa_node(hbus);
hv_pci_assign_slots(hbus);
pci_unlock_rescan_remove();
break;
@@ -2207,6 +2260,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
}
/**
+ * hv_pci_devices_present2() - Handle list of new children
+ * @hbus: Root PCI bus, as understood by this driver
+ * @relations: Packet from host listing children
+ *
+ * This function is the v2 version of hv_pci_devices_present()
+ */
+static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
+ struct pci_bus_relations2 *relations)
+{
+ struct hv_dr_state *dr;
+ int i;
+
+ dr = kzalloc(offsetof(struct hv_dr_state, func) +
+ (sizeof(struct hv_pcidev_description) *
+ (relations->device_count)), GFP_NOWAIT);
+
+ if (!dr)
+ return;
+
+ dr->device_count = relations->device_count;
+ for (i = 0; i < dr->device_count; i++) {
+ dr->func[i].v_id = relations->func[i].v_id;
+ dr->func[i].d_id = relations->func[i].d_id;
+ dr->func[i].rev = relations->func[i].rev;
+ dr->func[i].prog_intf = relations->func[i].prog_intf;
+ dr->func[i].subclass = relations->func[i].subclass;
+ dr->func[i].base_class = relations->func[i].base_class;
+ dr->func[i].subsystem_id = relations->func[i].subsystem_id;
+ dr->func[i].win_slot = relations->func[i].win_slot;
+ dr->func[i].ser = relations->func[i].ser;
+ dr->func[i].flags = relations->func[i].flags;
+ dr->func[i].virtual_numa_node =
+ relations->func[i].virtual_numa_node;
+ }
+
+ if (hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
+}
+
+/**
* hv_eject_device_work() - Asynchronously handles ejection
* @work: Work struct embedded in internal device struct
*
@@ -2319,6 +2412,7 @@ static void hv_pci_onchannelcallback(void *context)
struct pci_response *response;
struct pci_incoming_message *new_message;
struct pci_bus_relations *bus_rel;
+ struct pci_bus_relations2 *bus_rel2;
struct pci_dev_inval_block *inval;
struct pci_dev_incoming *dev_message;
struct hv_pci_dev *hpdev;
@@ -2386,6 +2480,21 @@ static void hv_pci_onchannelcallback(void *context)
hv_pci_devices_present(hbus, bus_rel);
break;
+ case PCI_BUS_RELATIONS2:
+
+ bus_rel2 = (struct pci_bus_relations2 *)buffer;
+ if (bytes_recvd <
+ offsetof(struct pci_bus_relations2, func) +
+ (sizeof(struct pci_function_description2) *
+ (bus_rel2->device_count))) {
+ dev_err(&hbus->hdev->device,
+ "bus relations v2 too small\n");
+ break;
+ }
+
+ hv_pci_devices_present2(hbus, bus_rel2);
+ break;
+
case PCI_EJECT:
dev_message = (struct pci_dev_incoming *)buffer;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net] vsock: fix potential deadlock in transport->release()
From: Stefano Garzarella @ 2020-02-26 10:58 UTC (permalink / raw)
To: davem
Cc: Dexuan Cui, Hillf Danton, virtualization, K. Y. Srinivasan, kvm,
Stephen Hemminger, syzbot+731710996d79d0d58fbc, netdev,
Sasha Levin, Sunil Muthuswamy, Stefano Garzarella, linux-kernel,
linux-hyperv, Jakub Kicinski, Haiyang Zhang, Stefan Hajnoczi,
Jorgen Hansen
Some transports (hyperv, virtio) acquire the sock lock during the
.release() callback.
In the vsock_stream_connect() we call vsock_assign_transport(); if
the socket was previously assigned to another transport, the
vsk->transport->release() is called, but the sock lock is already
held in the vsock_stream_connect(), causing a deadlock reported by
syzbot:
INFO: task syz-executor280:9768 blocked for more than 143 seconds.
Not tainted 5.6.0-rc1-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor280 D27912 9768 9766 0x00000000
Call Trace:
context_switch kernel/sched/core.c:3386 [inline]
__schedule+0x934/0x1f90 kernel/sched/core.c:4082
schedule+0xdc/0x2b0 kernel/sched/core.c:4156
__lock_sock+0x165/0x290 net/core/sock.c:2413
lock_sock_nested+0xfe/0x120 net/core/sock.c:2938
virtio_transport_release+0xc4/0xd60 net/vmw_vsock/virtio_transport_common.c:832
vsock_assign_transport+0xf3/0x3b0 net/vmw_vsock/af_vsock.c:454
vsock_stream_connect+0x2b3/0xc70 net/vmw_vsock/af_vsock.c:1288
__sys_connect_file+0x161/0x1c0 net/socket.c:1857
__sys_connect+0x174/0x1b0 net/socket.c:1874
__do_sys_connect net/socket.c:1885 [inline]
__se_sys_connect net/socket.c:1882 [inline]
__x64_sys_connect+0x73/0xb0 net/socket.c:1882
do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
To avoid this issue, this patch remove the lock acquiring in the
.release() callback of hyperv and virtio transports, and it holds
the lock when we call vsk->transport->release() in the vsock core.
Reported-by: syzbot+731710996d79d0d58fbc@syzkaller.appspotmail.com
Fixes: 408624af4c89 ("vsock: use local transport when it is loaded")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/af_vsock.c | 20 ++++++++++++--------
net/vmw_vsock/hyperv_transport.c | 3 ---
net/vmw_vsock/virtio_transport_common.c | 2 --
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 9c5b2a91baad..a5f28708e0e7 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -451,6 +451,12 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
if (vsk->transport == new_transport)
return 0;
+ /* transport->release() must be called with sock lock acquired.
+ * This path can only be taken during vsock_stream_connect(),
+ * where we have already held the sock lock.
+ * In the other cases, this function is called on a new socket
+ * which is not assigned to any transport.
+ */
vsk->transport->release(vsk);
vsock_deassign_transport(vsk);
}
@@ -753,20 +759,18 @@ static void __vsock_release(struct sock *sk, int level)
vsk = vsock_sk(sk);
pending = NULL; /* Compiler warning. */
- /* The release call is supposed to use lock_sock_nested()
- * rather than lock_sock(), if a sock lock should be acquired.
- */
- if (vsk->transport)
- vsk->transport->release(vsk);
- else if (sk->sk_type == SOCK_STREAM)
- vsock_remove_sock(vsk);
-
/* When "level" is SINGLE_DEPTH_NESTING, use the nested
* version to avoid the warning "possible recursive locking
* detected". When "level" is 0, lock_sock_nested(sk, level)
* is the same as lock_sock(sk).
*/
lock_sock_nested(sk, level);
+
+ if (vsk->transport)
+ vsk->transport->release(vsk);
+ else if (sk->sk_type == SOCK_STREAM)
+ vsock_remove_sock(vsk);
+
sock_orphan(sk);
sk->sk_shutdown = SHUTDOWN_MASK;
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 3492c021925f..630b851f8150 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -526,12 +526,9 @@ static bool hvs_close_lock_held(struct vsock_sock *vsk)
static void hvs_release(struct vsock_sock *vsk)
{
- struct sock *sk = sk_vsock(vsk);
bool remove_sock;
- lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
remove_sock = hvs_close_lock_held(vsk);
- release_sock(sk);
if (remove_sock)
vsock_remove_sock(vsk);
}
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index d9f0c9c5425a..f3c4bab2f737 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -829,7 +829,6 @@ void virtio_transport_release(struct vsock_sock *vsk)
struct sock *sk = &vsk->sk;
bool remove_sock = true;
- lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
if (sk->sk_type == SOCK_STREAM)
remove_sock = virtio_transport_close(vsk);
@@ -837,7 +836,6 @@ void virtio_transport_release(struct vsock_sock *vsk)
list_del(&pkt->list);
virtio_transport_free_pkt(pkt);
}
- release_sock(sk);
if (remove_sock)
vsock_remove_sock(vsk);
--
2.24.1
^ permalink raw reply related
* Re: [Patch v5 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message
From: Lorenzo Pieralisi @ 2020-02-26 11:47 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Andrew Murray, Bjorn Helgaas, linux-hyperv, linux-pci,
linux-kernel, Long Li
In-Reply-To: <1582693568-64759-1-git-send-email-longli@linuxonhyperv.com>
On Tue, Feb 25, 2020 at 09:06:07PM -0800, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> hv_dr_state is used to find present PCI devices on the bus. The structure
> reuses struct pci_function_description from VSP message to describe a
> device.
>
> To prepare support for pci_function_description v2, decouple this
> dependence in hv_dr_state so it can work with both v1 and v2 VSP messages.
>
> There is no functionality change.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> Changes
> v2: Changed some spaces to tabs, changed failure code to -ENOMEM
> v3: Revised comment for function hv_pci_devices_present(), reformatted patch title
> v4: Fixed spelling
> v5: Rebased to current tree
>
> drivers/pci/controller/pci-hyperv.c | 100 +++++++++++++++++++++++++-----------
> 1 file changed, 70 insertions(+), 30 deletions(-)
Applied the series to pci/hv for v5.7.
Thanks,
Lorenzo
>
> index 15011a3..dea197f 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -505,10 +505,24 @@ struct hv_dr_work {
> struct hv_pcibus_device *bus;
> };
>
> +struct hv_pcidev_description {
> + u16 v_id; /* vendor ID */
> + u16 d_id; /* device ID */
> + u8 rev;
> + u8 prog_intf;
> + u8 subclass;
> + u8 base_class;
> + u32 subsystem_id;
> + union win_slot_encoding win_slot;
> + u32 ser; /* serial number */
> + u32 flags;
> + u16 virtual_numa_node;
> +};
> +
> struct hv_dr_state {
> struct list_head list_entry;
> u32 device_count;
> - struct pci_function_description func[0];
> + struct hv_pcidev_description func[0];
> };
>
> enum hv_pcichild_state {
> @@ -525,7 +539,7 @@ struct hv_pci_dev {
> refcount_t refs;
> enum hv_pcichild_state state;
> struct pci_slot *pci_slot;
> - struct pci_function_description desc;
> + struct hv_pcidev_description desc;
> bool reported_missing;
> struct hv_pcibus_device *hbus;
> struct work_struct wrk;
> @@ -1877,7 +1891,7 @@ static void q_resource_requirements(void *context, struct pci_response *resp,
> * Return: Pointer to the new tracking struct
> */
> static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
> - struct pci_function_description *desc)
> + struct hv_pcidev_description *desc)
> {
> struct hv_pci_dev *hpdev;
> struct pci_child_message *res_req;
> @@ -1988,7 +2002,7 @@ static void pci_devices_present_work(struct work_struct *work)
> {
> u32 child_no;
> bool found;
> - struct pci_function_description *new_desc;
> + struct hv_pcidev_description *new_desc;
> struct hv_pci_dev *hpdev;
> struct hv_pcibus_device *hbus;
> struct list_head removed;
> @@ -2107,17 +2121,15 @@ static void pci_devices_present_work(struct work_struct *work)
> }
>
> /**
> - * hv_pci_devices_present() - Handles list of new children
> + * hv_pci_start_relations_work() - Queue work to start device discovery
> * @hbus: Root PCI bus, as understood by this driver
> - * @relations: Packet from host listing children
> + * @dr: The list of children returned from host
> *
> - * This function is invoked whenever a new list of devices for
> - * this bus appears.
> + * Return: 0 on success, -errno on failure
> */
> -static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> - struct pci_bus_relations *relations)
> +static int hv_pci_start_relations_work(struct hv_pcibus_device *hbus,
> + struct hv_dr_state *dr)
> {
> - struct hv_dr_state *dr;
> struct hv_dr_work *dr_wrk;
> unsigned long flags;
> bool pending_dr;
> @@ -2125,29 +2137,15 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> if (hbus->state == hv_pcibus_removing) {
> dev_info(&hbus->hdev->device,
> "PCI VMBus BUS_RELATIONS: ignored\n");
> - return;
> + return -ENOENT;
> }
>
> dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
> if (!dr_wrk)
> - return;
> -
> - dr = kzalloc(offsetof(struct hv_dr_state, func) +
> - (sizeof(struct pci_function_description) *
> - (relations->device_count)), GFP_NOWAIT);
> - if (!dr) {
> - kfree(dr_wrk);
> - return;
> - }
> + return -ENOMEM;
>
> INIT_WORK(&dr_wrk->wrk, pci_devices_present_work);
> dr_wrk->bus = hbus;
> - dr->device_count = relations->device_count;
> - if (dr->device_count != 0) {
> - memcpy(dr->func, relations->func,
> - sizeof(struct pci_function_description) *
> - dr->device_count);
> - }
>
> spin_lock_irqsave(&hbus->device_list_lock, flags);
> /*
> @@ -2165,6 +2163,47 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> get_hvpcibus(hbus);
> queue_work(hbus->wq, &dr_wrk->wrk);
> }
> +
> + return 0;
> +}
> +
> +/**
> + * hv_pci_devices_present() - Handle list of new children
> + * @hbus: Root PCI bus, as understood by this driver
> + * @relations: Packet from host listing children
> + *
> + * Process a new list of devices on the bus. The list of devices is
> + * discovered by VSP and sent to us via VSP message PCI_BUS_RELATIONS,
> + * whenever a new list of devices for this bus appears.
> + */
> +static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> + struct pci_bus_relations *relations)
> +{
> + struct hv_dr_state *dr;
> + int i;
> +
> + dr = kzalloc(offsetof(struct hv_dr_state, func) +
> + (sizeof(struct hv_pcidev_description) *
> + (relations->device_count)), GFP_NOWAIT);
> +
> + if (!dr)
> + return;
> +
> + dr->device_count = relations->device_count;
> + for (i = 0; i < dr->device_count; i++) {
> + dr->func[i].v_id = relations->func[i].v_id;
> + dr->func[i].d_id = relations->func[i].d_id;
> + dr->func[i].rev = relations->func[i].rev;
> + dr->func[i].prog_intf = relations->func[i].prog_intf;
> + dr->func[i].subclass = relations->func[i].subclass;
> + dr->func[i].base_class = relations->func[i].base_class;
> + dr->func[i].subsystem_id = relations->func[i].subsystem_id;
> + dr->func[i].win_slot = relations->func[i].win_slot;
> + dr->func[i].ser = relations->func[i].ser;
> + }
> +
> + if (hv_pci_start_relations_work(hbus, dr))
> + kfree(dr);
> }
>
> /**
> @@ -3069,7 +3108,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
> struct pci_packet teardown_packet;
> u8 buffer[sizeof(struct pci_message)];
> } pkt;
> - struct pci_bus_relations relations;
> + struct hv_dr_state *dr;
> struct hv_pci_compl comp_pkt;
> int ret;
>
> @@ -3082,8 +3121,9 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
>
> if (!hibernating) {
> /* Delete any children which might still exist. */
> - memset(&relations, 0, sizeof(relations));
> - hv_pci_devices_present(hbus, &relations);
> + dr = kzalloc(sizeof(*dr), GFP_KERNEL);
> + if (dr && hv_pci_start_relations_work(hbus, dr))
> + kfree(dr);
> }
>
> ret = hv_send_resources_released(hdev);
> --
> 1.8.3.1
>
^ permalink raw reply
* [PATCH] Hyper-V: add myself as a maintainer
From: Wei Liu @ 2020-02-26 18:01 UTC (permalink / raw)
To: linux-hyperv
Cc: Wei Liu, linux-kernel, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Michael Kelley
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
Cc: linux-kernel@vger.kernel.org
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Michael Kelley <mikelley@microsoft.com>
Sasha's entry hasn't been dropped from the Hyper-V tree yet, but that's
easy to resolve.
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8f27f40d22bb..ed943f205215 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7739,6 +7739,7 @@ M: "K. Y. Srinivasan" <kys@microsoft.com>
M: Haiyang Zhang <haiyangz@microsoft.com>
M: Stephen Hemminger <sthemmin@microsoft.com>
M: Sasha Levin <sashal@kernel.org>
+M: Wei Liu <wei.liu@kernel.org>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
L: linux-hyperv@vger.kernel.org
S: Supported
--
2.20.1
^ permalink raw reply related
* RE: [PATCH] Hyper-V: add myself as a maintainer
From: Haiyang Zhang @ 2020-02-26 18:57 UTC (permalink / raw)
To: Wei Liu, linux-hyperv@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
Michael Kelley
In-Reply-To: <20200226180102.16976-1-wei.liu@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]
> -----Original Message-----
> From: Wei Liu <wei.liu@kernel.org>
> Sent: Wednesday, February 26, 2020 1:01 PM
> To: linux-hyperv@vger.kernel.org
> Cc: Wei Liu <wei.liu@kernel.org>; linux-kernel@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley
> <mikelley@microsoft.com>
> Subject: [PATCH] Hyper-V: add myself as a maintainer
>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> Cc: linux-kernel@vger.kernel.org
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Michael Kelley <mikelley@microsoft.com>
>
> Sasha's entry hasn't been dropped from the Hyper-V tree yet, but that's easy to
> resolve.
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8f27f40d22bb..ed943f205215 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7739,6 +7739,7 @@ M: "K. Y. Srinivasan" <kys@microsoft.com>
> M: Haiyang Zhang <haiyangz@microsoft.com>
> M: Stephen Hemminger <sthemmin@microsoft.com>
> M: Sasha Levin <sashal@kernel.org>
> +M: Wei Liu <wei.liu@kernel.org>
Thanks for being a new maintainer for Hyper-V!
Sasha submitted a patch (attached) on 2/05 to remove himself from the
Hyper-V maintainer list. But his patch wasn't applied yet.
You may either re-submit his patch together with yours. Or just replace
his name / email with yours in one patch.
Thanks,
- Haiyang
[-- Attachment #2: Type: message/rfc822, Size: 6188 bytes --]
From: Sasha Levin <sashal@kernel.org>
To: "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>
Cc: Stephen Hemminger <sthemmin@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, KY Srinivasan <kys@microsoft.com>, Michael Kelley <mikelley@microsoft.com>, James Morris <James.Morris@microsoft.com>, Sasha Levin <sashal@kernel.org>
Subject: [PATCH] Hyper-V: Drop Sasha Levin from the Hyper-V maintainers
Date: Wed, 5 Feb 2020 21:32:42 +0000
Message-ID: <20200205213242.31343-1-sashal@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ed8bb8a1f5f..92cd55dfabe5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7732,7 +7732,6 @@ Hyper-V CORE AND DRIVERS
M: "K. Y. Srinivasan" <kys@microsoft.com>
M: Haiyang Zhang <haiyangz@microsoft.com>
M: Stephen Hemminger <sthemmin@microsoft.com>
-M: Sasha Levin <sashal@kernel.org>
T: git git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
L: linux-hyperv@vger.kernel.org
S: Supported
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] Hyper-V: add myself as a maintainer
From: Wei Liu @ 2020-02-26 19:06 UTC (permalink / raw)
To: Haiyang Zhang
Cc: Wei Liu, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
Michael Kelley
In-Reply-To: <MN2PR21MB1437C4BAAA6E38200C92F5DDCAEA0@MN2PR21MB1437.namprd21.prod.outlook.com>
On Wed, Feb 26, 2020 at 06:57:24PM +0000, Haiyang Zhang wrote:
>
>
> > -----Original Message-----
> > From: Wei Liu <wei.liu@kernel.org>
> > Sent: Wednesday, February 26, 2020 1:01 PM
> > To: linux-hyperv@vger.kernel.org
> > Cc: Wei Liu <wei.liu@kernel.org>; linux-kernel@vger.kernel.org; KY Srinivasan
> > <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> > Hemminger <sthemmin@microsoft.com>; Michael Kelley
> > <mikelley@microsoft.com>
> > Subject: [PATCH] Hyper-V: add myself as a maintainer
> >
> > Signed-off-by: Wei Liu <wei.liu@kernel.org>
> > ---
> > Cc: linux-kernel@vger.kernel.org
> > Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> > Cc: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > Cc: Michael Kelley <mikelley@microsoft.com>
> >
> > Sasha's entry hasn't been dropped from the Hyper-V tree yet, but that's easy to
> > resolve.
> > ---
> > MAINTAINERS | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8f27f40d22bb..ed943f205215 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7739,6 +7739,7 @@ M: "K. Y. Srinivasan" <kys@microsoft.com>
> > M: Haiyang Zhang <haiyangz@microsoft.com>
> > M: Stephen Hemminger <sthemmin@microsoft.com>
> > M: Sasha Levin <sashal@kernel.org>
> > +M: Wei Liu <wei.liu@kernel.org>
>
> Thanks for being a new maintainer for Hyper-V!
> Sasha submitted a patch (attached) on 2/05 to remove himself from the
> Hyper-V maintainer list. But his patch wasn't applied yet.
Yes I'm aware of his patch.
>
> You may either re-submit his patch together with yours. Or just replace
> his name / email with yours in one patch.
My plan is to apply his patch first and then apply this one once I get access
to hyperv tree. It is rather easy to resolve.
Wei.
^ permalink raw reply
* RE: [EXTERNAL] [PATCH] Hyper-V: add myself as a maintainer
From: KY Srinivasan @ 2020-02-27 15:18 UTC (permalink / raw)
To: Wei Liu, linux-hyperv@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Haiyang Zhang, Stephen Hemminger,
Michael Kelley
In-Reply-To: <20200226180102.16976-1-wei.liu@kernel.org>
> -----Original Message-----
> From: Wei Liu <wei.liu@kernel.org>
> Sent: Wednesday, February 26, 2020 10:01 AM
> To: linux-hyperv@vger.kernel.org
> Cc: Wei Liu <wei.liu@kernel.org>; linux-kernel@vger.kernel.org; KY Srinivasan
> <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley
> <mikelley@microsoft.com>
> Subject: [EXTERNAL] [PATCH] Hyper-V: add myself as a maintainer
>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
Acked-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> Cc: linux-kernel@vger.kernel.org
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Michael Kelley <mikelley@microsoft.com>
>
> Sasha's entry hasn't been dropped from the Hyper-V tree yet, but that's easy to
> resolve.
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8f27f40d22bb..ed943f205215 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7739,6 +7739,7 @@ M: "K. Y. Srinivasan" <kys@microsoft.com>
> M: Haiyang Zhang <haiyangz@microsoft.com>
> M: Stephen Hemminger <sthemmin@microsoft.com>
> M: Sasha Levin <sashal@kernel.org>
> +M: Wei Liu <wei.liu@kernel.org>
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
> L: linux-hyperv@vger.kernel.org
> S: Supported
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH net] vsock: fix potential deadlock in transport->release()
From: Stefan Hajnoczi @ 2020-02-27 16:11 UTC (permalink / raw)
To: Stefano Garzarella
Cc: davem, Dexuan Cui, Hillf Danton, virtualization, K. Y. Srinivasan,
kvm, Stephen Hemminger, syzbot+731710996d79d0d58fbc, netdev,
Sasha Levin, Sunil Muthuswamy, linux-kernel, linux-hyperv,
Jakub Kicinski, Haiyang Zhang, Jorgen Hansen
In-Reply-To: <20200226105818.36055-1-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]
On Wed, Feb 26, 2020 at 11:58:18AM +0100, Stefano Garzarella wrote:
> Some transports (hyperv, virtio) acquire the sock lock during the
> .release() callback.
>
> In the vsock_stream_connect() we call vsock_assign_transport(); if
> the socket was previously assigned to another transport, the
> vsk->transport->release() is called, but the sock lock is already
> held in the vsock_stream_connect(), causing a deadlock reported by
> syzbot:
>
> INFO: task syz-executor280:9768 blocked for more than 143 seconds.
> Not tainted 5.6.0-rc1-syzkaller #0
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> syz-executor280 D27912 9768 9766 0x00000000
> Call Trace:
> context_switch kernel/sched/core.c:3386 [inline]
> __schedule+0x934/0x1f90 kernel/sched/core.c:4082
> schedule+0xdc/0x2b0 kernel/sched/core.c:4156
> __lock_sock+0x165/0x290 net/core/sock.c:2413
> lock_sock_nested+0xfe/0x120 net/core/sock.c:2938
> virtio_transport_release+0xc4/0xd60 net/vmw_vsock/virtio_transport_common.c:832
> vsock_assign_transport+0xf3/0x3b0 net/vmw_vsock/af_vsock.c:454
> vsock_stream_connect+0x2b3/0xc70 net/vmw_vsock/af_vsock.c:1288
> __sys_connect_file+0x161/0x1c0 net/socket.c:1857
> __sys_connect+0x174/0x1b0 net/socket.c:1874
> __do_sys_connect net/socket.c:1885 [inline]
> __se_sys_connect net/socket.c:1882 [inline]
> __x64_sys_connect+0x73/0xb0 net/socket.c:1882
> do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> To avoid this issue, this patch remove the lock acquiring in the
> .release() callback of hyperv and virtio transports, and it holds
> the lock when we call vsk->transport->release() in the vsock core.
>
> Reported-by: syzbot+731710996d79d0d58fbc@syzkaller.appspotmail.com
> Fixes: 408624af4c89 ("vsock: use local transport when it is loaded")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/af_vsock.c | 20 ++++++++++++--------
> net/vmw_vsock/hyperv_transport.c | 3 ---
> net/vmw_vsock/virtio_transport_common.c | 2 --
> 3 files changed, 12 insertions(+), 13 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] vsock: fix potential deadlock in transport->release()
From: David Miller @ 2020-02-27 20:04 UTC (permalink / raw)
To: sgarzare
Cc: decui, hdanton, virtualization, kys, kvm, sthemmin,
syzbot+731710996d79d0d58fbc, netdev, sashal, sunilmut,
linux-kernel, linux-hyperv, kuba, haiyangz, stefanha, jhansen
In-Reply-To: <20200226105818.36055-1-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
Date: Wed, 26 Feb 2020 11:58:18 +0100
> Some transports (hyperv, virtio) acquire the sock lock during the
> .release() callback.
>
> In the vsock_stream_connect() we call vsock_assign_transport(); if
> the socket was previously assigned to another transport, the
> vsk->transport->release() is called, but the sock lock is already
> held in the vsock_stream_connect(), causing a deadlock reported by
> syzbot:
...
> To avoid this issue, this patch remove the lock acquiring in the
> .release() callback of hyperv and virtio transports, and it holds
> the lock when we call vsk->transport->release() in the vsock core.
>
> Reported-by: syzbot+731710996d79d0d58fbc@syzkaller.appspotmail.com
> Fixes: 408624af4c89 ("vsock: use local transport when it is loaded")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Applied, thank you.
^ permalink raw reply
* [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Lucas Tanure @ 2020-02-29 17:30 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Jiri Kosina, Benjamin Tissoires, linux-hyperv, linux-input,
linux-kernel
Cc: Lucas Tanure
Fix below warnings reported by coccicheck:
drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
Signed-off-by: Lucas Tanure <tanure@linux.com>
---
drivers/hid/hid-hyperv.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index dddfca555df9..0b6ee1dee625 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -193,8 +193,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
goto cleanup;
/* The pointer is not NULL when we resume from hibernation */
- if (input_device->hid_desc != NULL)
- kfree(input_device->hid_desc);
+ kfree(input_device->hid_desc);
input_device->hid_desc = kmemdup(desc, desc->bLength, GFP_ATOMIC);
if (!input_device->hid_desc)
@@ -207,8 +206,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
}
/* The pointer is not NULL when we resume from hibernation */
- if (input_device->report_desc != NULL)
- kfree(input_device->report_desc);
+ kfree(input_device->report_desc);
input_device->report_desc = kzalloc(input_device->report_desc_size,
GFP_ATOMIC);
--
2.25.1
^ permalink raw reply related
* RE: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Michael Kelley @ 2020-02-29 20:20 UTC (permalink / raw)
To: Lucas Tanure, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Jiri Kosina, Benjamin Tissoires,
linux-hyperv@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200229173007.61838-1-tanure@linux.com>
From: Lucas Tanure <tanure@linux.com> Sent: Saturday, February 29, 2020 9:30 AM
>
> Fix below warnings reported by coccicheck:
> drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is
> not needed.
> drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is
> not needed.
>
> Signed-off-by: Lucas Tanure <tanure@linux.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> drivers/hid/hid-hyperv.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index dddfca555df9..0b6ee1dee625 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -193,8 +193,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev
> *input_device,
> goto cleanup;
>
> /* The pointer is not NULL when we resume from hibernation */
> - if (input_device->hid_desc != NULL)
> - kfree(input_device->hid_desc);
> + kfree(input_device->hid_desc);
> input_device->hid_desc = kmemdup(desc, desc->bLength, GFP_ATOMIC);
>
> if (!input_device->hid_desc)
> @@ -207,8 +206,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev
> *input_device,
> }
>
> /* The pointer is not NULL when we resume from hibernation */
> - if (input_device->report_desc != NULL)
> - kfree(input_device->report_desc);
> + kfree(input_device->report_desc);
> input_device->report_desc = kzalloc(input_device->report_desc_size,
> GFP_ATOMIC);
>
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Wei Liu @ 2020-02-29 22:48 UTC (permalink / raw)
To: Lucas Tanure
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Jiri Kosina, Benjamin Tissoires, linux-hyperv, linux-input,
linux-kernel, Wei Liu
In-Reply-To: <20200229173007.61838-1-tanure@linux.com>
On Sat, Feb 29, 2020 at 05:30:07PM +0000, Lucas Tanure wrote:
> Fix below warnings reported by coccicheck:
> drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
> drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
>
> Signed-off-by: Lucas Tanure <tanure@linux.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
^ permalink raw reply
* Re: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Benjamin Tissoires @ 2020-03-02 10:16 UTC (permalink / raw)
To: Lucas Tanure
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Jiri Kosina, linux-hyperv, open list:HID CORE LAYER, lkml
In-Reply-To: <20200229173007.61838-1-tanure@linux.com>
On Sat, Feb 29, 2020 at 6:30 PM Lucas Tanure <tanure@linux.com> wrote:
>
> Fix below warnings reported by coccicheck:
> drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
> drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
>
> Signed-off-by: Lucas Tanure <tanure@linux.com>
> ---
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Sasha, do you prefer taking this through your tree or through the HID
one. I don't think we have much scheduled for hyperv, so it's up to
you.
Cheers,
Benjamin
> drivers/hid/hid-hyperv.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index dddfca555df9..0b6ee1dee625 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -193,8 +193,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
> goto cleanup;
>
> /* The pointer is not NULL when we resume from hibernation */
> - if (input_device->hid_desc != NULL)
> - kfree(input_device->hid_desc);
> + kfree(input_device->hid_desc);
> input_device->hid_desc = kmemdup(desc, desc->bLength, GFP_ATOMIC);
>
> if (!input_device->hid_desc)
> @@ -207,8 +206,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
> }
>
> /* The pointer is not NULL when we resume from hibernation */
> - if (input_device->report_desc != NULL)
> - kfree(input_device->report_desc);
> + kfree(input_device->report_desc);
> input_device->report_desc = kzalloc(input_device->report_desc_size,
> GFP_ATOMIC);
>
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Wei Liu @ 2020-03-02 12:09 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Lucas Tanure, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Jiri Kosina, linux-hyperv, open list:HID CORE LAYER,
lkml, Wei Liu
In-Reply-To: <CAO-hwJJDv=LnOQDbgWwg2sOccM9Tt-h=082Coi0aYdwG-CG-Kg@mail.gmail.com>
Hi Benjamin
On Mon, Mar 02, 2020 at 11:16:30AM +0100, Benjamin Tissoires wrote:
> On Sat, Feb 29, 2020 at 6:30 PM Lucas Tanure <tanure@linux.com> wrote:
> >
> > Fix below warnings reported by coccicheck:
> > drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
> > drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
> >
> > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > ---
>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Sasha, do you prefer taking this through your tree or through the HID
> one. I don't think we have much scheduled for hyperv, so it's up to
> you.
Sasha stepped down as a hyperv maintainer a few days back. I will be
taking over maintenance of the hyperv tree.
The problem is at the moment I haven't got write access to the
repository hosted on git.kernel.org. That's something I will need to
sort out as soon as possible.
In the meantime, it would be great if you can pick up this patch so that
it doesn't get lost while I sort out access on my side.
Thanks,
Wei.
^ permalink raw reply
* Re: [PATCH v1 1/3] x86/kvm/hyper-v: Add support for synthetic debugger capability
From: Vitaly Kuznetsov @ 2020-03-04 13:51 UTC (permalink / raw)
To: kvm; +Cc: Jon Doron, linux-hyperv
In-Reply-To: <20200303130356.50405-2-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> Add support for Hyper-V synthetic debugger (syndbg) interface.
> The syndbg interface is using MSRs to emulate a way to send/recv packets
> data.
>
> The debug transport dll (kdvm/kdnet) will identify if Hyper-V is enabled
> and if it supports the synthetic debugger interface it will attempt to
> use it, instead of trying to initialize a network adapter.
>
Cc: linux-hyperv@ list where Hyper-V folks live :-) They're in charge of
'hyperv-tlfs.h' so an ACK from them will be needed.
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 16 +++++
> arch/x86/include/asm/kvm_host.h | 11 +++
> arch/x86/kvm/hyperv.c | 109 +++++++++++++++++++++++++++++
> arch/x86/kvm/hyperv.h | 5 ++
> arch/x86/kvm/trace.h | 22 ++++++
> arch/x86/kvm/x86.c | 8 +++
> include/uapi/linux/kvm.h | 9 +++
> 7 files changed, 180 insertions(+)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 92abc1e42bfc..8efdf974c23f 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -33,6 +33,9 @@
> #define HYPERV_CPUID_ENLIGHTMENT_INFO 0x40000004
> #define HYPERV_CPUID_IMPLEMENT_LIMITS 0x40000005
> #define HYPERV_CPUID_NESTED_FEATURES 0x4000000A
> +#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS 0x40000080
> +#define HYPERV_CPUID_SYNDBG_INTERFACE 0x40000081
> +#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES 0x40000082
Out of pure curiosity, are these CPUIDs and MSRs documented somewhere?
I'm looking at TLFS v6.0 and failing to see them...
>
> #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
> #define HYPERV_CPUID_MIN 0x40000005
> @@ -131,6 +134,8 @@
> #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE BIT(8)
> /* Crash MSR available */
> #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10)
> +/* Support for debug MSRs available */
> +#define HV_FEATURE_DEBUG_MSRS_AVAILABLE BIT(11)
> /* stimer Direct Mode is available */
> #define HV_STIMER_DIRECT_MODE_AVAILABLE BIT(19)
>
> @@ -194,6 +199,9 @@
> #define HV_X64_NESTED_GUEST_MAPPING_FLUSH BIT(18)
> #define HV_X64_NESTED_MSR_BITMAP BIT(19)
>
> +/* Hyper-V synthetic debugger platform capabilities */
> +#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING BIT(1)
> +
> /* Hyper-V specific model specific registers (MSRs) */
>
> /* MSR used to identify the guest OS. */
> @@ -267,6 +275,14 @@
> /* Hyper-V guest idle MSR */
> #define HV_X64_MSR_GUEST_IDLE 0x400000F0
>
> +/* Hyper-V Synthetic debug options MSR */
> +#define HV_X64_MSR_SYNDBG_CONTROL 0x400000F1
> +#define HV_X64_MSR_SYNDBG_STATUS 0x400000F2
> +#define HV_X64_MSR_SYNDBG_SEND_BUFFER 0x400000F3
> +#define HV_X64_MSR_SYNDBG_RECV_BUFFER 0x400000F4
> +#define HV_X64_MSR_SYNDBG_PENDING_BUFFER 0x400000F5
> +#define HV_X64_MSR_SYNDBG_OPTIONS 0x400000FF
> +
> /* Hyper-V guest crash notification MSR's */
> #define HV_X64_MSR_CRASH_P0 0x40000100
> #define HV_X64_MSR_CRASH_P1 0x40000101
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 98959e8cd448..2b755174d683 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -854,6 +854,15 @@ struct kvm_apic_map {
> struct kvm_lapic *phys_map[];
> };
>
> +/* Hyper-V synthetic debugger (SynDbg)*/
> +struct kvm_hv_syndbg {
> + u64 control;
> + u64 status;
> + u64 send_page;
> + u64 recv_page;
> + u64 pending_page;
> +};
> +
> /* Hyper-V emulation context */
> struct kvm_hv {
> struct mutex hv_lock;
> @@ -877,6 +886,8 @@ struct kvm_hv {
> atomic_t num_mismatched_vp_indexes;
>
> struct hv_partition_assist_pg *hv_pa_pg;
> + u64 hv_syndbg_options;
> + struct kvm_hv_syndbg hv_syndbg;
I would've encapsulated both to struct kvm_hv_syndbg, e.g.
struct kvm_hv_syndbg {
struct {
u64 control;
u64 status;
u64 send_page;
u64 recv_page;
u64 pending_page;
} control;
u64 options;
}
To make it clear they're part of the same thing (are they?) I see you
handle HV_X64_MSR_SYNDBG_OPTIONS differently (outside of
syndbg_set_msr()).
> };
>
> enum kvm_irqchip_mode {
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index a86fda7a1d03..13176ec23496 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -266,6 +266,66 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
> return ret;
> }
>
> +static int kvm_hv_syndbg_complete_userspace(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_hv *hv = &kvm->arch.hyperv;
> +
> + if (vcpu->run->hyperv.u.syndbg.msr == HV_X64_MSR_SYNDBG_CONTROL)
> + hv->hv_syndbg.status = vcpu->run->hyperv.u.syndbg.status;
> + return 1;
> +}
> +
> +static void syndbg_exit(struct kvm_vcpu *vcpu, u32 msr)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> + struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
> +
> + hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNDBG;
> + hv_vcpu->exit.u.syndbg.msr = msr;
> + hv_vcpu->exit.u.syndbg.control = syndbg->control;
> + hv_vcpu->exit.u.syndbg.send_page = syndbg->send_page;
> + hv_vcpu->exit.u.syndbg.recv_page = syndbg->recv_page;
> + hv_vcpu->exit.u.syndbg.pending_page = syndbg->pending_page;
> + vcpu->arch.complete_userspace_io =
> + kvm_hv_syndbg_complete_userspace;
> +
> + kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
This new interface requires userspace support apparently so we can't
enable it unconditionally (userspaces which don't support it will be
very confused). You need to introduce a capability
(KVM_CAP_HYPERV_DEBUGGING?)
> +}
> +
> +static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> +{
> + struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> + int ret;
> +
> + trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id, msr, data);
> + ret = 0;
> + switch (msr) {
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + syndbg->control = data;
> + syndbg_exit(vcpu, msr);
> + break;
> + case HV_X64_MSR_SYNDBG_STATUS:
> + syndbg->status = data;
> + break;
> + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> + syndbg->send_page = data;
> + break;
> + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> + syndbg->recv_page = data;
> + break;
> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + syndbg->pending_page = data;
> + syndbg_exit(vcpu, msr);
> + break;
> + default:
> + ret = 1;
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
> bool host)
> {
> @@ -800,6 +860,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
> case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
> case HV_X64_MSR_TSC_EMULATION_CONTROL:
> case HV_X64_MSR_TSC_EMULATION_STATUS:
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> r = true;
> break;
> }
> @@ -1061,6 +1123,11 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
> if (!host)
> return 1;
> break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + hv->hv_syndbg_options = data;
> + break;
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + return syndbg_set_msr(vcpu, msr, data);
> default:
> vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
> msr, data);
> @@ -1227,6 +1294,24 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> case HV_X64_MSR_TSC_EMULATION_STATUS:
> data = hv->hv_tsc_emulation_status;
> break;
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> + data = hv->hv_syndbg_options;
> + break;
> + case HV_X64_MSR_SYNDBG_CONTROL:
> + data = hv->hv_syndbg.control;
> + break;
> + case HV_X64_MSR_SYNDBG_STATUS:
> + data = hv->hv_syndbg.status;
> + break;
> + case HV_X64_MSR_SYNDBG_SEND_BUFFER:
> + data = hv->hv_syndbg.send_page;
> + break;
> + case HV_X64_MSR_SYNDBG_RECV_BUFFER:
> + data = hv->hv_syndbg.recv_page;
> + break;
> + case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + data = hv->hv_syndbg.pending_page;
> + break;
> default:
> vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
> return 1;
> @@ -1797,6 +1882,9 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
> { .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> { .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> { .function = HYPERV_CPUID_NESTED_FEATURES },
> + { .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
> + { .function = HYPERV_CPUID_SYNDBG_INTERFACE },
> + { .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES },
HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS now returns
'HYPERV_CPUID_NESTED_FEATURES' as the last available leaf and I don't
see you adjusting it - is this expected?
> };
> int i, nent = ARRAY_SIZE(cpuid_entries);
>
> @@ -1856,9 +1944,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>
> ent->ebx |= HV_X64_POST_MESSAGES;
> ent->ebx |= HV_X64_SIGNAL_EVENTS;
> + ent->ebx |= HV_X64_DEBUGGING;
>
> ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
> + ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
> + ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
>
> /*
> * Direct Synthetic timers only make sense with in-kernel
> @@ -1903,6 +1994,24 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>
> break;
>
> + case HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS:
> + memcpy(signature, "Microsoft VS", 12);
Does this string matter? E.g. for HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS
we call ourselves "Linux KVM Hv", I think we should do the same here.
> +
> + ent->eax = 0;
> + ent->ebx = signature[0];
> + ent->ecx = signature[1];
> + ent->edx = signature[2];
> + break;
> +
> + case HYPERV_CPUID_SYNDBG_INTERFACE:
> + memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
> + ent->eax = signature[0];
> + break;
> +
> + case HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES:
> + ent->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
> + break;
> +
> default:
> break;
> }
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 757cb578101c..6a86151fac53 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -46,6 +46,11 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
> return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
> }
>
> +static inline struct kvm_hv_syndbg *vcpu_to_hv_syndbg(struct kvm_vcpu *vcpu)
> +{
> + return &vcpu->kvm->arch.hyperv.hv_syndbg;
> +}
> +
> int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
> int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
>
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index f194dd058470..235b9ab673a2 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -1515,6 +1515,28 @@ TRACE_EVENT(kvm_nested_vmenter_failed,
> __print_symbolic(__entry->err, VMX_VMENTER_INSTRUCTION_ERRORS))
> );
>
> +/*
> + * Tracepoint for syndbg_set_msr.
> + */
> +TRACE_EVENT(kvm_hv_syndbg_set_msr,
> + TP_PROTO(int vcpu_id, u32 msr, u64 data),
> + TP_ARGS(vcpu_id, msr, data),
> +
> + TP_STRUCT__entry(
> + __field(int, vcpu_id)
> + __field(u32, msr)
> + __field(u64, data)
> + ),
> +
> + TP_fast_assign(
> + __entry->vcpu_id = vcpu_id;
> + __entry->msr = msr;
> + __entry->data = data;
> + ),
> +
> + TP_printk("vcpu_id %d msr 0x%x data 0x%llx",
> + __entry->vcpu_id, __entry->msr, __entry->data)
This doesn't give us any additional data trace_kvm_msr_* points are more
or less the same. I think we can do better, e.g. for Hyper-V specific
things log the processor's VP index.
> +);
> #endif /* _TRACE_KVM_H */
>
> #undef TRACE_INCLUDE_PATH
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5de200663f51..9d4d72a88572 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1214,6 +1214,10 @@ static const u32 emulated_msrs_all[] = {
> HV_X64_MSR_VP_ASSIST_PAGE,
> HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
> HV_X64_MSR_TSC_EMULATION_STATUS,
> + HV_X64_MSR_SYNDBG_OPTIONS,
> + HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
> + HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
> + HV_X64_MSR_SYNDBG_PENDING_BUFFER,
>
> MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
> MSR_KVM_PV_EOI_EN,
> @@ -2906,6 +2910,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> */
> break;
> case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> case HV_X64_MSR_CRASH_CTL:
> case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
> @@ -3151,6 +3157,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> msr_info->data = 0x20000000;
> break;
> case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
> + case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
> + case HV_X64_MSR_SYNDBG_OPTIONS:
> case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
> case HV_X64_MSR_CRASH_CTL:
> case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b95f9a31a2f..fae8cf608976 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -188,6 +188,7 @@ struct kvm_s390_cmma_log {
> struct kvm_hyperv_exit {
> #define KVM_EXIT_HYPERV_SYNIC 1
> #define KVM_EXIT_HYPERV_HCALL 2
> +#define KVM_EXIT_HYPERV_SYNDBG 3
> __u32 type;
> union {
> struct {
> @@ -201,6 +202,14 @@ struct kvm_hyperv_exit {
> __u64 result;
> __u64 params[2];
> } hcall;
> + struct {
> + __u32 msr;
> + __u64 control;
> + __u64 status;
> + __u64 send_page;
> + __u64 recv_page;
> + __u64 pending_page;
> + } syndbg;
> } u;
> };
Not your fault but I just noticed that 'struct kvm_hyperv_exit' is not
properly padded. 'synic' struct is OK, however, 'hcall' is not as
there's gonna be a gap between '__u32 type' and it. Your 'struct syndbg'
is also OK as it starts with '__u32 msr' but we should do something
about hcall.
--
Vitaly
^ permalink raw reply
* Re: [PATCH v1 2/3] x86/kvm/hyper-v: enable hypercalls regardless of hypercall page
From: Vitaly Kuznetsov @ 2020-03-04 13:58 UTC (permalink / raw)
To: Jon Doron; +Cc: kvm, linux-hyperv
In-Reply-To: <20200303130356.50405-3-arilou@gmail.com>
Jon Doron <arilou@gmail.com> writes:
> Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
> page and simply identifies the CPU being used (AMD/Intel) and according
> to it simply makes hypercalls with the relevant instruction
> (vmmcall/vmcall respectively).
>
> The relevant function in kdvm is KdHvConnectHypervisor which first checks
> if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
> and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
> 0x1000101010001 which means:
> build_number = 0x0001
> service_version = 0x01
> minor_version = 0x01
> major_version = 0x01
> os_id = 0x00 (Undefined)
> vendor_id = 1 (Microsoft)
> os_type = 0 (A value of 0 indicates a proprietary, closed source OS)
>
> and starts issuing the hypercall without setting the hypercall page.
>
> To resolve this issue simply enable hypercalls if the guest_os_id is
> not 0.
>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
> arch/x86/kvm/hyperv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 13176ec23496..7ec962d433af 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1615,7 +1615,7 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
>
> bool kvm_hv_hypercall_enabled(struct kvm *kvm)
> {
> - return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
> + return READ_ONCE(kvm->arch.hyperv.hv_guest_os_id) != 0;
> }
>
> static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
I would've enabled it in both cases,
return (READ_ONCE(kvm->arch.hyperv.hv_hypercall) &
HV_X64_MSR_HYPERCALL_ENABLE) || (READ_ONCE(kvm->arch.hyperv.hv_guest_os_id) != 0);
to be safe. We can also check what genuine Hyper-V does but I bet it has
hypercalls always enabled. Also, the function can be made inline,
there's a single caller.
--
Vitaly
^ permalink raw reply
* Re: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Wei Liu @ 2020-03-04 14:48 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Lucas Tanure, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Jiri Kosina, linux-hyperv, open list:HID CORE LAYER,
lkml, Wei Liu
In-Reply-To: <20200302120951.fhdafzl5xtnmjrls@debian>
On Mon, Mar 02, 2020 at 12:09:51PM +0000, Wei Liu wrote:
> Hi Benjamin
>
> On Mon, Mar 02, 2020 at 11:16:30AM +0100, Benjamin Tissoires wrote:
> > On Sat, Feb 29, 2020 at 6:30 PM Lucas Tanure <tanure@linux.com> wrote:
> > >
> > > Fix below warnings reported by coccicheck:
> > > drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
> > > drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
> > >
> > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > ---
> >
> > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > Sasha, do you prefer taking this through your tree or through the HID
> > one. I don't think we have much scheduled for hyperv, so it's up to
> > you.
>
> Sasha stepped down as a hyperv maintainer a few days back. I will be
> taking over maintenance of the hyperv tree.
>
> The problem is at the moment I haven't got write access to the
> repository hosted on git.kernel.org. That's something I will need to
> sort out as soon as possible.
>
> In the meantime, it would be great if you can pick up this patch so that
> it doesn't get lost while I sort out access on my side.
Hi Benjamin
I got access to the Hyper-V tree. I will be picking this patch up since
I haven't got a confirmation from your side.
Wei.
>
> Thanks,
> Wei.
^ permalink raw reply
* Re: [PATCH] HID: hyperv: NULL check before some freeing functions is not needed.
From: Benjamin Tissoires @ 2020-03-04 14:55 UTC (permalink / raw)
To: Wei Liu
Cc: Lucas Tanure, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Jiri Kosina, linux-hyperv, open list:HID CORE LAYER,
lkml
In-Reply-To: <20200304144858.xc6ekcvbzrhbggsc@debian>
On Wed, Mar 4, 2020 at 3:49 PM Wei Liu <wei.liu@kernel.org> wrote:
>
> On Mon, Mar 02, 2020 at 12:09:51PM +0000, Wei Liu wrote:
> > Hi Benjamin
> >
> > On Mon, Mar 02, 2020 at 11:16:30AM +0100, Benjamin Tissoires wrote:
> > > On Sat, Feb 29, 2020 at 6:30 PM Lucas Tanure <tanure@linux.com> wrote:
> > > >
> > > > Fix below warnings reported by coccicheck:
> > > > drivers/hid/hid-hyperv.c:197:2-7: WARNING: NULL check before some freeing functions is not needed.
> > > > drivers/hid/hid-hyperv.c:211:2-7: WARNING: NULL check before some freeing functions is not needed.
> > > >
> > > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > > ---
> > >
> > > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > >
> > > Sasha, do you prefer taking this through your tree or through the HID
> > > one. I don't think we have much scheduled for hyperv, so it's up to
> > > you.
> >
> > Sasha stepped down as a hyperv maintainer a few days back. I will be
> > taking over maintenance of the hyperv tree.
> >
> > The problem is at the moment I haven't got write access to the
> > repository hosted on git.kernel.org. That's something I will need to
> > sort out as soon as possible.
> >
> > In the meantime, it would be great if you can pick up this patch so that
> > it doesn't get lost while I sort out access on my side.
>
> Hi Benjamin
>
> I got access to the Hyper-V tree. I will be picking this patch up since
> I haven't got a confirmation from your side.
>
Great, thanks.
Sorry, I am pulled in freedesktop tasks right now that are a little bit urgent.
Glad you quickly set up the access rights.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
From: Wei Liu @ 2020-03-04 17:55 UTC (permalink / raw)
To: Dexuan Cui, Lorenzo Pieralisi
Cc: Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Wei Liu
In-Reply-To: <HK0P153MB0148FB68FCBAE908CA5991C3BF1A0@HK0P153MB0148.APCP153.PROD.OUTLOOK.COM>
On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > From: linux-hyperv-owner@vger.kernel.org
> > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > Sent: Wednesday, February 12, 2020 4:51 PM
> > ...
> > The current codebase makes use of the zero-length array language
> > extension to the C90 standard, but the preferred mechanism to declare
> > variable-length types such as these ones is a flexible array member[1][2],
> > introduced in C99:
> >
> > struct foo {
> > int stuff;
> > struct boo array[];
> > };
> >
> > By making use of the mechanism above, we will get a compiler warning
> > in case the flexible array does not occur last in the structure, which
> > will help us prevent some kind of undefined behavior bugs from being
> > inadvertently introduced[3] to the codebase from now on.
> >
> > Also, notice that, dynamic memory allocations won't be affected by
> > this change:
> >
> > "Flexible array members have incomplete type, and so the sizeof operator
> > may not be applied. As a quirk of the original implementation of
> > zero-length arrays, sizeof evaluates to zero."[1]
> >
> > This issue was found with the help of Coccinelle.
>
> Looks good to me. Thanks, Gustavo!
>
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
>
Lorenzo, will you be picking up this patch? It seems to me you've been
handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
in your repository.
Let me know what you think.
Wei.
^ permalink raw reply
* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
From: Lorenzo Pieralisi @ 2020-03-04 18:06 UTC (permalink / raw)
To: Wei Liu
Cc: Dexuan Cui, Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Andrew Murray, Bjorn Helgaas,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200304175509.dwhn63omfzewaukv@debian>
On Wed, Mar 04, 2020 at 05:55:09PM +0000, Wei Liu wrote:
> On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > > From: linux-hyperv-owner@vger.kernel.org
> > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > > Sent: Wednesday, February 12, 2020 4:51 PM
> > > ...
> > > The current codebase makes use of the zero-length array language
> > > extension to the C90 standard, but the preferred mechanism to declare
> > > variable-length types such as these ones is a flexible array member[1][2],
> > > introduced in C99:
> > >
> > > struct foo {
> > > int stuff;
> > > struct boo array[];
> > > };
> > >
> > > By making use of the mechanism above, we will get a compiler warning
> > > in case the flexible array does not occur last in the structure, which
> > > will help us prevent some kind of undefined behavior bugs from being
> > > inadvertently introduced[3] to the codebase from now on.
> > >
> > > Also, notice that, dynamic memory allocations won't be affected by
> > > this change:
> > >
> > > "Flexible array members have incomplete type, and so the sizeof operator
> > > may not be applied. As a quirk of the original implementation of
> > > zero-length arrays, sizeof evaluates to zero."[1]
> > >
> > > This issue was found with the help of Coccinelle.
> >
> > Looks good to me. Thanks, Gustavo!
> >
> > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> >
>
> Lorenzo, will you be picking up this patch? It seems to me you've been
> handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
> in your repository.
>
> Let me know what you think.
I shall pick it up, I checked patchwork and it was erroneously
assigned to Bjorn, that's why I have not taken it yet.
Fixed now, apologies, I will merge it shortly.
Lorenzo
^ permalink raw reply
* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
From: Wei Liu @ 2020-03-04 18:10 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Wei Liu, Dexuan Cui, Gustavo A. R. Silva, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Andrew Murray,
Bjorn Helgaas, linux-hyperv@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200304180635.GA21844@e121166-lin.cambridge.arm.com>
On Wed, Mar 04, 2020 at 06:06:35PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Mar 04, 2020 at 05:55:09PM +0000, Wei Liu wrote:
> > On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > > > From: linux-hyperv-owner@vger.kernel.org
> > > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > > > Sent: Wednesday, February 12, 2020 4:51 PM
> > > > ...
> > > > The current codebase makes use of the zero-length array language
> > > > extension to the C90 standard, but the preferred mechanism to declare
> > > > variable-length types such as these ones is a flexible array member[1][2],
> > > > introduced in C99:
> > > >
> > > > struct foo {
> > > > int stuff;
> > > > struct boo array[];
> > > > };
> > > >
> > > > By making use of the mechanism above, we will get a compiler warning
> > > > in case the flexible array does not occur last in the structure, which
> > > > will help us prevent some kind of undefined behavior bugs from being
> > > > inadvertently introduced[3] to the codebase from now on.
> > > >
> > > > Also, notice that, dynamic memory allocations won't be affected by
> > > > this change:
> > > >
> > > > "Flexible array members have incomplete type, and so the sizeof operator
> > > > may not be applied. As a quirk of the original implementation of
> > > > zero-length arrays, sizeof evaluates to zero."[1]
> > > >
> > > > This issue was found with the help of Coccinelle.
> > >
> > > Looks good to me. Thanks, Gustavo!
> > >
> > > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> > >
> >
> > Lorenzo, will you be picking up this patch? It seems to me you've been
> > handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
> > in your repository.
> >
> > Let me know what you think.
>
> I shall pick it up, I checked patchwork and it was erroneously
> assigned to Bjorn, that's why I have not taken it yet.
>
> Fixed now, apologies, I will merge it shortly.
Thanks for picking it up.
Wei.
>
> Lorenzo
^ permalink raw reply
* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
From: Gustavo A. R. Silva @ 2020-03-04 18:24 UTC (permalink / raw)
To: Wei Liu, Lorenzo Pieralisi
Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, Andrew Murray, Bjorn Helgaas,
linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200304181017.epqvhmtegefb4eba@debian>
On 3/4/20 12:10, Wei Liu wrote:
>>>>
>>>> Looks good to me. Thanks, Gustavo!
>>>>
>>>> Reviewed-by: Dexuan Cui <decui@microsoft.com>
>>>>
>>>
>>> Lorenzo, will you be picking up this patch? It seems to me you've been
>>> handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
>>> in your repository.
>>>
>>> Let me know what you think.
>>
>> I shall pick it up, I checked patchwork and it was erroneously
>> assigned to Bjorn, that's why I have not taken it yet.
>>
>> Fixed now, apologies, I will merge it shortly.
>
> Thanks for picking it up.
>
Thank you all, guys. :)
--
Gustavo
^ permalink raw reply
page: next (older) | 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