[PATCH v4 0/7] xen-pciback: misc cleanup

[PATCH v4 0/7] xen-pciback: misc cleanup

[Jan Beulich]

The first five patches are the result of the requested split of
"xen-pciback: clean up {bar,rom}_init()", with Boris'es R-b
dropped despite there not being any functional change (the
mechanical change appears too significant to retain it). The
remaining two are a follow-up to the recent "xen/pciback: Fix
conf_space read/write overlap check." and I hope the folding
in of the formatting correction of two lines touched anyway is
going to be acceptable this time.

1: drop unused function parameter of read_dev_bar()
2: drop rom_init() 
3: fold read_dev_bar() into its now single caller
4: simplify determination of 64-bit memory resource
5: use const and unsigned in bar_init()
6: short-circuit read path used for merging write values
7: drop superfluous variables

Signed-off-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 1/7] xen-pciback: drop unused function parameter of read_dev_bar()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space_header.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space_header.c
@@ -210,8 +210,7 @@ static int bar_read(struct pci_dev *dev,
 }
 
 static inline void read_dev_bar(struct pci_dev *dev,
-				struct pci_bar_info *bar_info, int offset,
-				u32 len_mask)
+				struct pci_bar_info *bar_info, int offset)
 {
 	int	pos;
 	struct resource	*res = dev->resource;
@@ -248,7 +247,7 @@ static void *bar_init(struct pci_dev *de
 	if (!bar)
 		return ERR_PTR(-ENOMEM);
 
-	read_dev_bar(dev, bar, offset, ~0);
+	read_dev_bar(dev, bar, offset);
 
 	return bar;
 }
@@ -260,7 +259,7 @@ static void *rom_init(struct pci_dev *de
 	if (!bar)
 		return ERR_PTR(-ENOMEM);
 
-	read_dev_bar(dev, bar, offset, ~PCI_ROM_ADDRESS_ENABLE);
+	read_dev_bar(dev, bar, offset);
 
 	return bar;
 }




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 2/7] xen-pciback: drop rom_init()

[Jan Beulich]

It is now identical to bar_init().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space_header.c |   14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space_header.c
@@ -252,18 +252,6 @@ static void *bar_init(struct pci_dev *de
 	return bar;
 }
 
-static void *rom_init(struct pci_dev *dev, int offset)
-{
-	struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
-
-	if (!bar)
-		return ERR_PTR(-ENOMEM);
-
-	read_dev_bar(dev, bar, offset);
-
-	return bar;
-}
-
 static void bar_reset(struct pci_dev *dev, int offset, void *data)
 {
 	struct pci_bar_info *bar = data;
@@ -382,7 +370,7 @@ static const struct config_field header_
 	{						\
 	.offset     = reg_offset,			\
 	.size       = 4,				\
-	.init       = rom_init,				\
+	.init       = bar_init,				\
 	.reset      = bar_reset,			\
 	.release    = bar_release,			\
 	.u.dw.read  = bar_read,				\




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 3/7] xen-pciback: fold read_dev_bar() into its now single caller

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space_header.c |   33 +++++++++++-----------------
 1 file changed, 13 insertions(+), 20 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space_header.c
@@ -209,11 +209,14 @@ static int bar_read(struct pci_dev *dev,
 	return 0;
 }
 
-static inline void read_dev_bar(struct pci_dev *dev,
-				struct pci_bar_info *bar_info, int offset)
+static void *bar_init(struct pci_dev *dev, int offset)
 {
 	int	pos;
 	struct resource	*res = dev->resource;
+	struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
+
+	if (!bar)
+		return ERR_PTR(-ENOMEM);
 
 	if (offset == PCI_ROM_ADDRESS || offset == PCI_ROM_ADDRESS1)
 		pos = PCI_ROM_RESOURCE;
@@ -223,31 +226,21 @@ static inline void read_dev_bar(struct p
 				PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
 			   (PCI_BASE_ADDRESS_SPACE_MEMORY |
 				PCI_BASE_ADDRESS_MEM_TYPE_64))) {
-			bar_info->val = res[pos - 1].start >> 32;
-			bar_info->len_val = -resource_size(&res[pos - 1]) >> 32;
-			return;
+			bar->val = res[pos - 1].start >> 32;
+			bar->len_val = -resource_size(&res[pos - 1]) >> 32;
+			return bar;
 		}
 	}
 
 	if (!res[pos].flags ||
 	    (res[pos].flags & (IORESOURCE_DISABLED | IORESOURCE_UNSET |
 			       IORESOURCE_BUSY)))
-		return;
-
-	bar_info->val = res[pos].start |
-			(res[pos].flags & PCI_REGION_FLAG_MASK);
-	bar_info->len_val = -resource_size(&res[pos]) |
-			    (res[pos].flags & PCI_REGION_FLAG_MASK);
-}
-
-static void *bar_init(struct pci_dev *dev, int offset)
-{
-	struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
-
-	if (!bar)
-		return ERR_PTR(-ENOMEM);
+		return bar;
 
-	read_dev_bar(dev, bar, offset);
+	bar->val = res[pos].start |
+		   (res[pos].flags & PCI_REGION_FLAG_MASK);
+	bar->len_val = -resource_size(&res[pos]) |
+		       (res[pos].flags & PCI_REGION_FLAG_MASK);
 
 	return bar;
 }




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 4/7] xen-pciback: simplify determination of 64-bit memory resource

[Jan Beulich]

Other than for raw BAR values, flags are properly separated in the
internal representation.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/xen/xen-pciback/conf_space_header.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space_header.c
@@ -222,10 +222,7 @@ static void *bar_init(struct pci_dev *de
 		pos = PCI_ROM_RESOURCE;
 	else {
 		pos = (offset - PCI_BASE_ADDRESS_0) / 4;
-		if (pos && ((res[pos - 1].flags & (PCI_BASE_ADDRESS_SPACE |
-				PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
-			   (PCI_BASE_ADDRESS_SPACE_MEMORY |
-				PCI_BASE_ADDRESS_MEM_TYPE_64))) {
+		if (pos && (res[pos - 1].flags & IORESOURCE_MEM_64)) {
 			bar->val = res[pos - 1].start >> 32;
 			bar->len_val = -resource_size(&res[pos - 1]) >> 32;
 			return bar;




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 5/7] xen-pciback: use const and unsigned in bar_init()

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space_header.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space_header.c
@@ -211,8 +211,8 @@ static int bar_read(struct pci_dev *dev,
 
 static void *bar_init(struct pci_dev *dev, int offset)
 {
-	int	pos;
-	struct resource	*res = dev->resource;
+	unsigned int pos;
+	const struct resource *res = dev->resource;
 	struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
 
 	if (!bar)




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 6/7] xen-pciback: short-circuit read path used for merging write values

[Jan Beulich]

There's no point calling xen_pcibk_config_read() here - all it'll do is
return whatever conf_space_read() returns for the field which was found
here (and which would be found there again). Also there's no point
clearing tmp_val before the call.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space.c
@@ -230,10 +230,8 @@ int xen_pcibk_config_write(struct pci_de
 		field_end = OFFSET(cfg_entry) + field->size;
 
 		 if (req_end > field_start && field_end > req_start) {
-			tmp_val = 0;
-
-			err = xen_pcibk_config_read(dev, field_start,
-						  field->size, &tmp_val);
+			err = conf_space_read(dev, cfg_entry, field_start,
+					      &tmp_val);
 			if (err)
 				break;
 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 7/7] xen-pciback: drop superfluous variables

[Jan Beulich]

req_start is simply an alias of the "offset" function parameter, and
req_end is being used just once in each function. (And both variables
were loop invariant anyway, so should at least have got initialized
outside the loop.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 drivers/xen/xen-pciback/conf_space.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

--- 4.7-rc6-xen-pciback.orig/drivers/xen/xen-pciback/conf_space.c
+++ 4.7-rc6-xen-pciback/drivers/xen/xen-pciback/conf_space.c
@@ -148,7 +148,7 @@ int xen_pcibk_config_read(struct pci_dev
 	struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(dev);
 	const struct config_field_entry *cfg_entry;
 	const struct config_field *field;
-	int req_start, req_end, field_start, field_end;
+	int field_start, field_end;
 	/* if read fails for any reason, return 0
 	 * (as if device didn't respond) */
 	u32 value = 0, tmp_val;
@@ -178,12 +178,10 @@ int xen_pcibk_config_read(struct pci_dev
 	list_for_each_entry(cfg_entry, &dev_data->config_fields, list) {
 		field = cfg_entry->field;
 
-		req_start = offset;
-		req_end = offset + size;
 		field_start = OFFSET(cfg_entry);
 		field_end = OFFSET(cfg_entry) + field->size;
 
-		 if (req_end > field_start && field_end > req_start) {
+		if (offset + size > field_start && field_end > offset) {
 			err = conf_space_read(dev, cfg_entry, field_start,
 					      &tmp_val);
 			if (err)
@@ -191,7 +189,7 @@ int xen_pcibk_config_read(struct pci_dev
 
 			value = merge_value(value, tmp_val,
 					    get_mask(field->size),
-					    field_start - req_start);
+					    field_start - offset);
 		}
 	}
 
@@ -211,7 +209,7 @@ int xen_pcibk_config_write(struct pci_de
 	const struct config_field_entry *cfg_entry;
 	const struct config_field *field;
 	u32 tmp_val;
-	int req_start, req_end, field_start, field_end;
+	int field_start, field_end;
 
 	if (unlikely(verbose_request))
 		printk(KERN_DEBUG
@@ -224,19 +222,17 @@ int xen_pcibk_config_write(struct pci_de
 	list_for_each_entry(cfg_entry, &dev_data->config_fields, list) {
 		field = cfg_entry->field;
 
-		req_start = offset;
-		req_end = offset + size;
 		field_start = OFFSET(cfg_entry);
 		field_end = OFFSET(cfg_entry) + field->size;
 
-		 if (req_end > field_start && field_end > req_start) {
+		if (offset + size > field_start && field_end > offset) {
 			err = conf_space_read(dev, cfg_entry, field_start,
 					      &tmp_val);
 			if (err)
 				break;
 
 			tmp_val = merge_value(tmp_val, value, get_mask(size),
-					      req_start - field_start);
+					      offset - field_start);
 
 			err = conf_space_write(dev, cfg_entry, field_start,
 					       tmp_val);



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 0/7] xen-pciback: misc cleanup

[David Vrabel]

On 06/07/16 07:47, Jan Beulich wrote:
> The first five patches are the result of the requested split of
> "xen-pciback: clean up {bar,rom}_init()", with Boris'es R-b
> dropped despite there not being any functional change (the
> mechanical change appears too significant to retain it). The
> remaining two are a follow-up to the recent "xen/pciback: Fix
> conf_space read/write overlap check." and I hope the folding
> in of the formatting correction of two lines touched anyway is
> going to be acceptable this time.

Applied to for-linus-4.8, thanks.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel