diff mbox series

[3/3] vfio/pci: Expose setup ROM at ROM bar when needed

Message ID 20241212205050.5737-3-Yunxiang.Li@amd.com (mailing list archive)
State New
Headers show
Series [1/3] vfio/pci: Remove shadow rom specific code paths | expand

Commit Message

Li, Yunxiang (Teddy) Dec. 12, 2024, 8:50 p.m. UTC 
If ROM bar is missing for any reason, we can fallback to using pdev->rom
to expose the ROM content to the guest. This fixes some passthrough use
cases where the upstream bridge does not have enough address window.

Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
---
 drivers/vfio/pci/vfio_pci_config.c |  4 ++++
 drivers/vfio/pci/vfio_pci_core.c   | 35 +++++++++++++++---------------
 drivers/vfio/pci/vfio_pci_rdwr.c   | 14 ++++++++++--
 3 files changed, 34 insertions(+), 19 deletions(-)

Comments

Alex Williamson Dec. 12, 2024, 11 p.m. UTC | #1 
On Thu, 12 Dec 2024 15:50:50 -0500
Yunxiang Li <Yunxiang.Li@amd.com> wrote:

> If ROM bar is missing for any reason, we can fallback to using pdev->rom
> to expose the ROM content to the guest. This fixes some passthrough use
> cases where the upstream bridge does not have enough address window.
> 
> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> ---
>  drivers/vfio/pci/vfio_pci_config.c |  4 ++++
>  drivers/vfio/pci/vfio_pci_core.c   | 35 +++++++++++++++---------------
>  drivers/vfio/pci/vfio_pci_rdwr.c   | 14 ++++++++++--
>  3 files changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index e41c3a965663e..4c673d842fb35 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -511,6 +511,10 @@ static void vfio_bar_fixup(struct vfio_pci_core_device *vdev)
>  		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
>  		mask |= PCI_ROM_ADDRESS_ENABLE;
>  		*vbar &= cpu_to_le32((u32)mask);
> +	} else if (pdev->rom && pdev->romlen) {
> +		mask = ~(roundup_pow_of_two(pdev->romlen) - 1);
> +		mask |= PCI_ROM_ADDRESS_ENABLE;
> +		*vbar &= cpu_to_le32((u32)mask);
>  	} else {
>  		*vbar = 0;
>  	}
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index b49dd9cdc072a..3120c1e9f22cb 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1049,30 +1049,31 @@ static int vfio_pci_ioctl_get_region_info(struct vfio_pci_core_device *vdev,
>  		break;
>  	case VFIO_PCI_ROM_REGION_INDEX: {
>  		void __iomem *io;
> -		size_t size;
> +		size_t dont_care;

It still receives the size even if we don't consume it.

>  		u16 cmd;
>  
>  		info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
>  		info.flags = 0;
> +		info.size = 0;
>  
> -		/* Report the BAR size, not the ROM size */
> -		info.size = pci_resource_len(pdev, info.index);
> -		if (!info.size)
> -			break;
> -
> -		/*
> -		 * Is it really there?  Enable memory decode for implicit access
> -		 * in pci_map_rom().
> -		 */
> -		cmd = vfio_pci_memory_lock_and_enable(vdev);
> -		io = pci_map_rom(pdev, &size);
> -		if (io) {
> +		if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
> +			/* Check ROM content is valid. Need to enable memory

Incorrect comment style.

> +			 * decode for ROM access in pci_map_rom().
> +			 */
> +			cmd = vfio_pci_memory_lock_and_enable(vdev);
> +			io = pci_map_rom(pdev, &dont_care);
> +			if (io) {
> +				info.flags = VFIO_REGION_INFO_FLAG_READ;
> +				/* Report the BAR size, not the ROM size. */
> +				info.size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
> +				pci_unmap_rom(pdev, io);
> +			}
> +			vfio_pci_memory_unlock_and_restore(vdev, cmd);
> +		} else if (pdev->rom && pdev->romlen) {
>  			info.flags = VFIO_REGION_INFO_FLAG_READ;
> -			pci_unmap_rom(pdev, io);
> -		} else {
> -			info.size = 0;
> +			/* Report BAR size as power of two. */
> +			info.size = roundup_pow_of_two(pdev->romlen);
>  		}
> -		vfio_pci_memory_unlock_and_restore(vdev, cmd);
>  
>  		break;
>  	}
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index 4bed9fd5af50f..4ea983cf499d9 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> @@ -243,6 +243,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  
>  	if (pci_resource_start(pdev, bar))
>  		end = pci_resource_len(pdev, bar);
> +	else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
> +		end = roundup_pow_of_two(pdev->romlen);
>  	else
>  		return -EINVAL;
>  
> @@ -259,7 +261,12 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  		 * excluded range at the end of the actual ROM.  This makes
>  		 * filling large ROM BARs much faster.
>  		 */
> -		io = pci_map_rom(pdev, &x_start);
> +		if (pci_resource_start(pdev, bar)) {
> +			io = pci_map_rom(pdev, &x_start);
> +		} else {
> +			io = ioremap(pdev->rom, pdev->romlen);
> +			x_start = pdev->romlen;
> +		}
>  		if (!io)
>  			return -ENOMEM;
>  		x_end = end;
> @@ -267,7 +274,10 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  		done = vfio_pci_core_do_io_rw(vdev, 1, io, buf, pos,
>  					      count, x_start, x_end, 0);
>  
> -		pci_unmap_rom(pdev, io);
> +		if (pci_resource_start(pdev, bar))
> +			pci_unmap_rom(pdev, io);
> +		else
> +			iounmap(io);
>  	} else {
>  		done = vfio_pci_core_setup_barmap(vdev, bar);
>  		if (done)

It's not clear why the refactoring of the previous patch was really
necessary simply to have an alternate map and unmap path.  Thanks,

Alex
diff mbox series

Patch

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index e41c3a965663e..4c673d842fb35 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -511,6 +511,10 @@  static void vfio_bar_fixup(struct vfio_pci_core_device *vdev)
 		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
 		mask |= PCI_ROM_ADDRESS_ENABLE;
 		*vbar &= cpu_to_le32((u32)mask);
+	} else if (pdev->rom && pdev->romlen) {
+		mask = ~(roundup_pow_of_two(pdev->romlen) - 1);
+		mask |= PCI_ROM_ADDRESS_ENABLE;
+		*vbar &= cpu_to_le32((u32)mask);
 	} else {
 		*vbar = 0;
 	}
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index b49dd9cdc072a..3120c1e9f22cb 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1049,30 +1049,31 @@  static int vfio_pci_ioctl_get_region_info(struct vfio_pci_core_device *vdev,
 		break;
 	case VFIO_PCI_ROM_REGION_INDEX: {
 		void __iomem *io;
-		size_t size;
+		size_t dont_care;
 		u16 cmd;
 
 		info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
 		info.flags = 0;
+		info.size = 0;
 
-		/* Report the BAR size, not the ROM size */
-		info.size = pci_resource_len(pdev, info.index);
-		if (!info.size)
-			break;
-
-		/*
-		 * Is it really there?  Enable memory decode for implicit access
-		 * in pci_map_rom().
-		 */
-		cmd = vfio_pci_memory_lock_and_enable(vdev);
-		io = pci_map_rom(pdev, &size);
-		if (io) {
+		if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
+			/* Check ROM content is valid. Need to enable memory
+			 * decode for ROM access in pci_map_rom().
+			 */
+			cmd = vfio_pci_memory_lock_and_enable(vdev);
+			io = pci_map_rom(pdev, &dont_care);
+			if (io) {
+				info.flags = VFIO_REGION_INFO_FLAG_READ;
+				/* Report the BAR size, not the ROM size. */
+				info.size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
+				pci_unmap_rom(pdev, io);
+			}
+			vfio_pci_memory_unlock_and_restore(vdev, cmd);
+		} else if (pdev->rom && pdev->romlen) {
 			info.flags = VFIO_REGION_INFO_FLAG_READ;
-			pci_unmap_rom(pdev, io);
-		} else {
-			info.size = 0;
+			/* Report BAR size as power of two. */
+			info.size = roundup_pow_of_two(pdev->romlen);
 		}
-		vfio_pci_memory_unlock_and_restore(vdev, cmd);
 
 		break;
 	}
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 4bed9fd5af50f..4ea983cf499d9 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -243,6 +243,8 @@  ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 
 	if (pci_resource_start(pdev, bar))
 		end = pci_resource_len(pdev, bar);
+	else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
+		end = roundup_pow_of_two(pdev->romlen);
 	else
 		return -EINVAL;
 
@@ -259,7 +261,12 @@  ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 		 * excluded range at the end of the actual ROM.  This makes
 		 * filling large ROM BARs much faster.
 		 */
-		io = pci_map_rom(pdev, &x_start);
+		if (pci_resource_start(pdev, bar)) {
+			io = pci_map_rom(pdev, &x_start);
+		} else {
+			io = ioremap(pdev->rom, pdev->romlen);
+			x_start = pdev->romlen;
+		}
 		if (!io)
 			return -ENOMEM;
 		x_end = end;
@@ -267,7 +274,10 @@  ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 		done = vfio_pci_core_do_io_rw(vdev, 1, io, buf, pos,
 					      count, x_start, x_end, 0);
 
-		pci_unmap_rom(pdev, io);
+		if (pci_resource_start(pdev, bar))
+			pci_unmap_rom(pdev, io);
+		else
+			iounmap(io);
 	} else {
 		done = vfio_pci_core_setup_barmap(vdev, bar);
 		if (done)