| Message ID | 20200609221610.42862-2-ian.may@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | LP:#1880975 Request to include two NUMA related commits in Azure kernels | expand |
On 10.06.20 00:16, Ian May wrote: > From: Long Li <longli@microsoft.com> > > BugLink: https://bugs.launchpad.net/bugs/1880975 > > 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> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reviewed-by: Michael Kelley <mikelley@microsoft.com> > (backported from f9ad0f361cf3b58fd26d409c6150126547259772) > [since hibernate patches are not in tree, needed to manually apply part of patch] > Signed-off-by: Ian May <ian.may@canonical.com> > --- It is probably just the wording you used which somehow throw me off. You more or less adjusted for different context. Why it is different usually is not that important and for some reason made me initially believe some parts of the hybernation patches might be added. > drivers/pci/controller/pci-hyperv.c | 98 ++++++++++++++++++++--------- > 1 file changed, 69 insertions(+), 29 deletions(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index 3a56de6b2ec2..01efdfedefc5 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -474,10 +474,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 { > @@ -494,7 +508,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; > @@ -1579,7 +1593,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; > @@ -1690,7 +1704,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; > @@ -1809,41 +1823,25 @@ 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; > > 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); > /* > @@ -1861,6 +1859,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); > } > > /** > @@ -2711,7 +2750,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev) > 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; > > @@ -2723,8 +2762,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev) > return; > > /* 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); > if (ret) >
Thanks Stefan! I'll keep that in mind moving forward. I'm still trying to find that balance of saying enough without saying too much! Thanks Ian On 6/10/20 3:00 AM, Stefan Bader wrote: > On 10.06.20 00:16, Ian May wrote: >> From: Long Li <longli@microsoft.com> >> >> BugLink: https://bugs.launchpad.net/bugs/1880975 >> >> 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> >> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> Reviewed-by: Michael Kelley <mikelley@microsoft.com> >> (backported from f9ad0f361cf3b58fd26d409c6150126547259772) >> [since hibernate patches are not in tree, needed to manually apply part of patch] >> Signed-off-by: Ian May <ian.may@canonical.com> >> --- > It is probably just the wording you used which somehow throw me off. You more or > less adjusted for different context. Why it is different usually is not that > important and for some reason made me initially believe some parts of the > hybernation patches might be added. > >> drivers/pci/controller/pci-hyperv.c | 98 ++++++++++++++++++++--------- >> 1 file changed, 69 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c >> index 3a56de6b2ec2..01efdfedefc5 100644 >> --- a/drivers/pci/controller/pci-hyperv.c >> +++ b/drivers/pci/controller/pci-hyperv.c >> @@ -474,10 +474,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 { >> @@ -494,7 +508,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; >> @@ -1579,7 +1593,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; >> @@ -1690,7 +1704,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; >> @@ -1809,41 +1823,25 @@ 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; >> >> 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); >> /* >> @@ -1861,6 +1859,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); >> } >> >> /** >> @@ -2711,7 +2750,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev) >> 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; >> >> @@ -2723,8 +2762,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev) >> return; >> >> /* 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); >> if (ret) >> >
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 3a56de6b2ec2..01efdfedefc5 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -474,10 +474,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 { @@ -494,7 +508,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; @@ -1579,7 +1593,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; @@ -1690,7 +1704,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; @@ -1809,41 +1823,25 @@ 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; 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); /* @@ -1861,6 +1859,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); } /** @@ -2711,7 +2750,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev) 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; @@ -2723,8 +2762,9 @@ static void hv_pci_bus_exit(struct hv_device *hdev) return; /* 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); if (ret)