* [PATCH] kernel vfio: PCI ROM size calculation
@ 2012-05-12 6:50 ` Alexey Kardashevskiy
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kardashevskiy @ 2012-05-12 6:50 UTC (permalink / raw)
To: Alex Williamson
Cc: Benjamin Herrenschmidt, Alex Williamson, David Gibson, Alex Graf,
kvm, qemu-devel
The original code calculated the real size of the ROM and passed it to
QEMU which tried to do pci_register_bar and failed with error message
"ERROR: PCI region size must be pow2".
The example card is PCIe Intel E1000E with the ROM size 0x9c00.
pci_resource_len seems to be a proper solution.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/pci/vfio_pci.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b2f1f3a..6d4a783 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
* this gives the user an easy way to determine whether
* there's anything here w/o trying to read it. */
if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
- void __iomem *io;
- size_t size;
-
- io = pci_map_rom(pdev, &size);
- info.size = io ? size : 0;
- pci_unmap_rom(pdev, io);
+ info.size = pci_resource_len(pdev,
+ PCI_ROM_RESOURCE);
+ info.flags |= VFIO_PCI_ROM_REGION_INDEX;
} else if (flags & IORESOURCE_MEM) {
info.size = pci_resource_len(pdev, info.index);
info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
--
Alexey
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
@ 2012-05-12 6:50 ` Alexey Kardashevskiy
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kardashevskiy @ 2012-05-12 6:50 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, qemu-devel, Alex Graf, David Gibson
The original code calculated the real size of the ROM and passed it to
QEMU which tried to do pci_register_bar and failed with error message
"ERROR: PCI region size must be pow2".
The example card is PCIe Intel E1000E with the ROM size 0x9c00.
pci_resource_len seems to be a proper solution.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/pci/vfio_pci.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b2f1f3a..6d4a783 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
* this gives the user an easy way to determine whether
* there's anything here w/o trying to read it. */
if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
- void __iomem *io;
- size_t size;
-
- io = pci_map_rom(pdev, &size);
- info.size = io ? size : 0;
- pci_unmap_rom(pdev, io);
+ info.size = pci_resource_len(pdev,
+ PCI_ROM_RESOURCE);
+ info.flags |= VFIO_PCI_ROM_REGION_INDEX;
} else if (flags & IORESOURCE_MEM) {
info.size = pci_resource_len(pdev, info.index);
info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
--
Alexey
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel vfio: PCI ROM size calculation
2012-05-12 6:50 ` [Qemu-devel] " Alexey Kardashevskiy
@ 2012-05-14 2:55 ` Alex Williamson
-1 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2012-05-14 2:55 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: David Gibson, qemu-devel, kvm, Alex Graf
On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
> The original code calculated the real size of the ROM and passed it to
> QEMU which tried to do pci_register_bar and failed with error message
> "ERROR: PCI region size must be pow2".
>
> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
>
> pci_resource_len seems to be a proper solution.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> drivers/vfio/pci/vfio_pci.c | 9 +++------
> 1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index b2f1f3a..6d4a783 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
> * this gives the user an easy way to determine whether
> * there's anything here w/o trying to read it. */
This is actually intentional, but maybe isn't the best way to handle it
anymore. The cut-off comment above reads:
/* Report the actual ROM size instead of the BAR size,
* this gives the user an easy way to determine whether
* there's anything here w/o trying to read it. */
At the time I added that though, I think we still had a separate ioctl
to get the BAR size, so one gave the real size, the other the BAR size.
Do we care to expose that ROMs have real size, maybe zero, that doesn't
necessarily match pci_resource_len()? Thanks,
Alex
> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
> - void __iomem *io;
> - size_t size;
> -
> - io = pci_map_rom(pdev, &size);
> - info.size = io ? size : 0;
> - pci_unmap_rom(pdev, io);
> + info.size = pci_resource_len(pdev,
> + PCI_ROM_RESOURCE);
> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
> } else if (flags & IORESOURCE_MEM) {
> info.size = pci_resource_len(pdev, info.index);
> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
@ 2012-05-14 2:55 ` Alex Williamson
0 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2012-05-14 2:55 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: David Gibson, qemu-devel, kvm, Alex Graf
On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
> The original code calculated the real size of the ROM and passed it to
> QEMU which tried to do pci_register_bar and failed with error message
> "ERROR: PCI region size must be pow2".
>
> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
>
> pci_resource_len seems to be a proper solution.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> drivers/vfio/pci/vfio_pci.c | 9 +++------
> 1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index b2f1f3a..6d4a783 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
> * this gives the user an easy way to determine whether
> * there's anything here w/o trying to read it. */
This is actually intentional, but maybe isn't the best way to handle it
anymore. The cut-off comment above reads:
/* Report the actual ROM size instead of the BAR size,
* this gives the user an easy way to determine whether
* there's anything here w/o trying to read it. */
At the time I added that though, I think we still had a separate ioctl
to get the BAR size, so one gave the real size, the other the BAR size.
Do we care to expose that ROMs have real size, maybe zero, that doesn't
necessarily match pci_resource_len()? Thanks,
Alex
> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
> - void __iomem *io;
> - size_t size;
> -
> - io = pci_map_rom(pdev, &size);
> - info.size = io ? size : 0;
> - pci_unmap_rom(pdev, io);
> + info.size = pci_resource_len(pdev,
> + PCI_ROM_RESOURCE);
> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
> } else if (flags & IORESOURCE_MEM) {
> info.size = pci_resource_len(pdev, info.index);
> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
2012-05-14 2:55 ` [Qemu-devel] " Alex Williamson
@ 2012-05-14 3:24 ` Alexey Kardashevskiy
-1 siblings, 0 replies; 8+ messages in thread
From: Alexey Kardashevskiy @ 2012-05-14 3:24 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, qemu-devel, Alex Graf, David Gibson
On 14/05/12 12:55, Alex Williamson wrote:
> On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
>> The original code calculated the real size of the ROM and passed it to
>> QEMU which tried to do pci_register_bar and failed with error message
>> "ERROR: PCI region size must be pow2".
>>
>> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
>>
>> pci_resource_len seems to be a proper solution.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> drivers/vfio/pci/vfio_pci.c | 9 +++------
>> 1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index b2f1f3a..6d4a783 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
>> * this gives the user an easy way to determine whether
>> * there's anything here w/o trying to read it. */
>
> This is actually intentional, but maybe isn't the best way to handle it
> anymore. The cut-off comment above reads:
>
> /* Report the actual ROM size instead of the BAR size,
> * this gives the user an easy way to determine whether
> * there's anything here w/o trying to read it. */
>
> At the time I added that though, I think we still had a separate ioctl
> to get the BAR size, so one gave the real size, the other the BAR size.
> Do we care to expose that ROMs have real size, maybe zero, that doesn't
> necessarily match pci_resource_len()? Thanks,
The guest will still see aligned ROM BAR (not real one) and may try to read behind the real end of
ROM, and it is allowed while it is still within the BAR, right?
>
> Alex
>
>> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
>> - void __iomem *io;
>> - size_t size;
>> -
>> - io = pci_map_rom(pdev, &size);
>> - info.size = io ? size : 0;
>> - pci_unmap_rom(pdev, io);
>> + info.size = pci_resource_len(pdev,
>> + PCI_ROM_RESOURCE);
>> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
>> } else if (flags & IORESOURCE_MEM) {
>> info.size = pci_resource_len(pdev, info.index);
>> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
>>
>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
@ 2012-05-14 3:24 ` Alexey Kardashevskiy
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kardashevskiy @ 2012-05-14 3:24 UTC (permalink / raw)
To: Alex Williamson; +Cc: David Gibson, qemu-devel, kvm, Alex Graf
On 14/05/12 12:55, Alex Williamson wrote:
> On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
>> The original code calculated the real size of the ROM and passed it to
>> QEMU which tried to do pci_register_bar and failed with error message
>> "ERROR: PCI region size must be pow2".
>>
>> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
>>
>> pci_resource_len seems to be a proper solution.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> drivers/vfio/pci/vfio_pci.c | 9 +++------
>> 1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index b2f1f3a..6d4a783 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
>> * this gives the user an easy way to determine whether
>> * there's anything here w/o trying to read it. */
>
> This is actually intentional, but maybe isn't the best way to handle it
> anymore. The cut-off comment above reads:
>
> /* Report the actual ROM size instead of the BAR size,
> * this gives the user an easy way to determine whether
> * there's anything here w/o trying to read it. */
>
> At the time I added that though, I think we still had a separate ioctl
> to get the BAR size, so one gave the real size, the other the BAR size.
> Do we care to expose that ROMs have real size, maybe zero, that doesn't
> necessarily match pci_resource_len()? Thanks,
The guest will still see aligned ROM BAR (not real one) and may try to read behind the real end of
ROM, and it is allowed while it is still within the BAR, right?
>
> Alex
>
>> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
>> - void __iomem *io;
>> - size_t size;
>> -
>> - io = pci_map_rom(pdev, &size);
>> - info.size = io ? size : 0;
>> - pci_unmap_rom(pdev, io);
>> + info.size = pci_resource_len(pdev,
>> + PCI_ROM_RESOURCE);
>> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
>> } else if (flags & IORESOURCE_MEM) {
>> info.size = pci_resource_len(pdev, info.index);
>> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
>>
>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
2012-05-14 3:24 ` Alexey Kardashevskiy
@ 2012-05-14 4:34 ` Alex Williamson
-1 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2012-05-14 4:34 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: David Gibson, qemu-devel, kvm, Alex Graf
On Mon, 2012-05-14 at 13:24 +1000, Alexey Kardashevskiy wrote:
> On 14/05/12 12:55, Alex Williamson wrote:
> > On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
> >> The original code calculated the real size of the ROM and passed it to
> >> QEMU which tried to do pci_register_bar and failed with error message
> >> "ERROR: PCI region size must be pow2".
> >>
> >> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
> >>
> >> pci_resource_len seems to be a proper solution.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >> drivers/vfio/pci/vfio_pci.c | 9 +++------
> >> 1 files changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> >> index b2f1f3a..6d4a783 100644
> >> --- a/drivers/vfio/pci/vfio_pci.c
> >> +++ b/drivers/vfio/pci/vfio_pci.c
> >> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
> >> * this gives the user an easy way to determine whether
> >> * there's anything here w/o trying to read it. */
> >
> > This is actually intentional, but maybe isn't the best way to handle it
> > anymore. The cut-off comment above reads:
> >
> > /* Report the actual ROM size instead of the BAR size,
> > * this gives the user an easy way to determine whether
> > * there's anything here w/o trying to read it. */
> >
> > At the time I added that though, I think we still had a separate ioctl
> > to get the BAR size, so one gave the real size, the other the BAR size.
> > Do we care to expose that ROMs have real size, maybe zero, that doesn't
> > necessarily match pci_resource_len()? Thanks,
>
>
> The guest will still see aligned ROM BAR (not real one) and may try to read behind the real end of
> ROM, and it is allowed while it is still within the BAR, right?
Yes.
> >> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
> >> - void __iomem *io;
> >> - size_t size;
> >> -
> >> - io = pci_map_rom(pdev, &size);
> >> - info.size = io ? size : 0;
> >> - pci_unmap_rom(pdev, io);
> >> + info.size = pci_resource_len(pdev,
> >> + PCI_ROM_RESOURCE);
> >> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
> >> } else if (flags & IORESOURCE_MEM) {
> >> info.size = pci_resource_len(pdev, info.index);
> >> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
> >>
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] kernel vfio: PCI ROM size calculation
@ 2012-05-14 4:34 ` Alex Williamson
0 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2012-05-14 4:34 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Alex Graf, qemu-devel, kvm, David Gibson
On Mon, 2012-05-14 at 13:24 +1000, Alexey Kardashevskiy wrote:
> On 14/05/12 12:55, Alex Williamson wrote:
> > On Sat, 2012-05-12 at 16:50 +1000, Alexey Kardashevskiy wrote:
> >> The original code calculated the real size of the ROM and passed it to
> >> QEMU which tried to do pci_register_bar and failed with error message
> >> "ERROR: PCI region size must be pow2".
> >>
> >> The example card is PCIe Intel E1000E with the ROM size 0x9c00.
> >>
> >> pci_resource_len seems to be a proper solution.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >> drivers/vfio/pci/vfio_pci.c | 9 +++------
> >> 1 files changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> >> index b2f1f3a..6d4a783 100644
> >> --- a/drivers/vfio/pci/vfio_pci.c
> >> +++ b/drivers/vfio/pci/vfio_pci.c
> >> @@ -234,12 +234,9 @@ static long vfio_pci_ioctl(void *device_data,
> >> * this gives the user an easy way to determine whether
> >> * there's anything here w/o trying to read it. */
> >
> > This is actually intentional, but maybe isn't the best way to handle it
> > anymore. The cut-off comment above reads:
> >
> > /* Report the actual ROM size instead of the BAR size,
> > * this gives the user an easy way to determine whether
> > * there's anything here w/o trying to read it. */
> >
> > At the time I added that though, I think we still had a separate ioctl
> > to get the BAR size, so one gave the real size, the other the BAR size.
> > Do we care to expose that ROMs have real size, maybe zero, that doesn't
> > necessarily match pci_resource_len()? Thanks,
>
>
> The guest will still see aligned ROM BAR (not real one) and may try to read behind the real end of
> ROM, and it is allowed while it is still within the BAR, right?
Yes.
> >> if (info.index == VFIO_PCI_ROM_REGION_INDEX) {
> >> - void __iomem *io;
> >> - size_t size;
> >> -
> >> - io = pci_map_rom(pdev, &size);
> >> - info.size = io ? size : 0;
> >> - pci_unmap_rom(pdev, io);
> >> + info.size = pci_resource_len(pdev,
> >> + PCI_ROM_RESOURCE);
> >> + info.flags |= VFIO_PCI_ROM_REGION_INDEX;
> >> } else if (flags & IORESOURCE_MEM) {
> >> info.size = pci_resource_len(pdev, info.index);
> >> info.flags |= (VFIO_REGION_INFO_FLAG_WRITE |
> >>
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-05-14 4:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-12 6:50 [PATCH] kernel vfio: PCI ROM size calculation Alexey Kardashevskiy
2012-05-12 6:50 ` [Qemu-devel] " Alexey Kardashevskiy
2012-05-14 2:55 ` Alex Williamson
2012-05-14 2:55 ` [Qemu-devel] " Alex Williamson
2012-05-14 3:24 ` Alexey Kardashevskiy
2012-05-14 3:24 ` Alexey Kardashevskiy
2012-05-14 4:34 ` Alex Williamson
2012-05-14 4:34 ` Alex Williamson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.