[eoan:linux-azure][PATCH 1/2] PCI: hv: Decouple the func definition in hv_dr_state from VSP message

Ian May ian.may at canonical.com
Wed Jun 10 13:26:59 UTC 2020


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 at 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 at microsoft.com>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
>> Reviewed-by: Michael Kelley <mikelley at 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 at 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)
>>
>




More information about the kernel-team mailing list