| Message ID | 20191125103033.22694-5-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add writable BARs and PCIE 1.1 support | expand |
On Mon, 25 Nov 2019 10:30:21 +0000 Alexandru Elisei <alexandru.elisei@arm.com> wrote: Hi, > According to the PCI local bus specification [1], a device's memory size > must be a power of two. This is also implicit in the mechanism that a CPU > uses to get the memory size requirement for a PCI device. > > The vesa device requests a memory size that isn't a power of two. > According to the same spec [1], a device is allowed to consume more memory > than it actually requires. As a result, the amount of memory that the vesa > device now reserves has been increased. > > To prevent slip-ups in the future, a few BUILD_BUG_ON statements were added > in places where the memory size is known at compile time. > > [1] PCI Local Bus Specification Revision 3.0, section 6.2.5.1 > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > hw/vesa.c | 2 ++ > include/kvm/util.h | 2 ++ > include/kvm/vesa.h | 6 +++++- > vfio/pci.c | 5 +++++ > virtio/pci.c | 3 +++ > 5 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/hw/vesa.c b/hw/vesa.c > index f3c5114cf4fe..75670a51be5f 100644 > --- a/hw/vesa.c > +++ b/hw/vesa.c > @@ -58,6 +58,8 @@ struct framebuffer *vesa__init(struct kvm *kvm) > char *mem; > int r; > > + BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); > + > if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) > return NULL; > > diff --git a/include/kvm/util.h b/include/kvm/util.h > index 4ca7aa9392b6..e90f1c2db39f 100644 > --- a/include/kvm/util.h > +++ b/include/kvm/util.h > @@ -104,6 +104,8 @@ static inline unsigned long roundup_pow_of_two(unsigned long x) > return x ? 1UL << fls_long(x - 1) : 0; > } > > +#define is_power_of_two(x) ((x) ? ((x) & ((x) - 1)) == 0 : 0) This gives weird results for negative values (which the kernel avoids by having this a static inline and using an unsigned type). Not sure we care, but maybe (x > 0) ? ... would fix this? > + > struct kvm; > void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size); > void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size); > diff --git a/include/kvm/vesa.h b/include/kvm/vesa.h > index 0fac11ab5a9f..e7d971343642 100644 > --- a/include/kvm/vesa.h > +++ b/include/kvm/vesa.h > @@ -5,8 +5,12 @@ > #define VESA_HEIGHT 480 > > #define VESA_MEM_ADDR 0xd0000000 > -#define VESA_MEM_SIZE (4*VESA_WIDTH*VESA_HEIGHT) > #define VESA_BPP 32 > +/* > + * We actually only need VESA_BPP/8*VESA_WIDTH*VESA_HEIGHT bytes. But the memory > + * size must be a power of 2, so we round up. > + */ > +#define VESA_MEM_SIZE (1 << 21) I don't think it's worth calculating the value and rounding it up, but can we have a BUILD_BUG checking that VESA_MEM_SIZE is big enough to hold the framebuffer? Cheers, Andre > > struct kvm; > struct biosregs; > diff --git a/vfio/pci.c b/vfio/pci.c > index 76e24c156906..914732cc6897 100644 > --- a/vfio/pci.c > +++ b/vfio/pci.c > @@ -831,6 +831,11 @@ static int vfio_pci_configure_bar(struct kvm *kvm, struct vfio_device *vdev, > /* Ignore invalid or unimplemented regions */ > if (!region->info.size) > return 0; > + if (!is_power_of_two(region->info.size)) { > + vfio_dev_err(vdev, "region is not power of two: 0x%llx", > + region->info.size); > + return -EINVAL; > + } > > if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_MSIX) { > /* Trap and emulate MSI-X table */ > diff --git a/virtio/pci.c b/virtio/pci.c > index 99653cad2c0f..04e801827df9 100644 > --- a/virtio/pci.c > +++ b/virtio/pci.c > @@ -435,6 +435,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev, > vpci->kvm = kvm; > vpci->dev = dev; > > + BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE)); > + BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE)); > + > r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vdev); > if (r < 0) > return r;
Hi, On 11/27/19 6:25 PM, Andre Przywara wrote: > On Mon, 25 Nov 2019 10:30:21 +0000 > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi, > >> According to the PCI local bus specification [1], a device's memory size >> must be a power of two. This is also implicit in the mechanism that a CPU >> uses to get the memory size requirement for a PCI device. >> >> The vesa device requests a memory size that isn't a power of two. >> According to the same spec [1], a device is allowed to consume more memory >> than it actually requires. As a result, the amount of memory that the vesa >> device now reserves has been increased. >> >> To prevent slip-ups in the future, a few BUILD_BUG_ON statements were added >> in places where the memory size is known at compile time. >> >> [1] PCI Local Bus Specification Revision 3.0, section 6.2.5.1 >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >> --- >> hw/vesa.c | 2 ++ >> include/kvm/util.h | 2 ++ >> include/kvm/vesa.h | 6 +++++- >> vfio/pci.c | 5 +++++ >> virtio/pci.c | 3 +++ >> 5 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/hw/vesa.c b/hw/vesa.c >> index f3c5114cf4fe..75670a51be5f 100644 >> --- a/hw/vesa.c >> +++ b/hw/vesa.c >> @@ -58,6 +58,8 @@ struct framebuffer *vesa__init(struct kvm *kvm) >> char *mem; >> int r; >> >> + BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); >> + >> if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) >> return NULL; >> >> diff --git a/include/kvm/util.h b/include/kvm/util.h >> index 4ca7aa9392b6..e90f1c2db39f 100644 >> --- a/include/kvm/util.h >> +++ b/include/kvm/util.h >> @@ -104,6 +104,8 @@ static inline unsigned long roundup_pow_of_two(unsigned long x) >> return x ? 1UL << fls_long(x - 1) : 0; >> } >> >> +#define is_power_of_two(x) ((x) ? ((x) & ((x) - 1)) == 0 : 0) > This gives weird results for negative values (which the kernel avoids by having this a static inline and using an unsigned type). > Not sure we care, but maybe (x > 0) ? ... would fix this? Good point, I will fix it by changing the implicit comparison against 0 at the beginning with (x) > 0. > >> + >> struct kvm; >> void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size); >> void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size); >> diff --git a/include/kvm/vesa.h b/include/kvm/vesa.h >> index 0fac11ab5a9f..e7d971343642 100644 >> --- a/include/kvm/vesa.h >> +++ b/include/kvm/vesa.h >> @@ -5,8 +5,12 @@ >> #define VESA_HEIGHT 480 >> >> #define VESA_MEM_ADDR 0xd0000000 >> -#define VESA_MEM_SIZE (4*VESA_WIDTH*VESA_HEIGHT) >> #define VESA_BPP 32 >> +/* >> + * We actually only need VESA_BPP/8*VESA_WIDTH*VESA_HEIGHT bytes. But the memory >> + * size must be a power of 2, so we round up. >> + */ >> +#define VESA_MEM_SIZE (1 << 21) > I don't think it's worth calculating the value and rounding it up, but can we have a BUILD_BUG checking that VESA_MEM_SIZE is big enough to hold the framebuffer? I'm not sure what you mean. The current value of VESA_MEM_SIZE is not a power of two, which breaks the spec. That's the reason for changing it. Are you suggesting that we keep VESA_MEM_SIZE = 1 << 21, we remove the power_of_two check and add a BUILD_BUG on VESA_MEM_SIZE < 4 * VESA_WIDTH * VESA_HEIGHT? Thanks, Alex > > Cheers, > Andre > >> >> struct kvm; >> struct biosregs; >> diff --git a/vfio/pci.c b/vfio/pci.c >> index 76e24c156906..914732cc6897 100644 >> --- a/vfio/pci.c >> +++ b/vfio/pci.c >> @@ -831,6 +831,11 @@ static int vfio_pci_configure_bar(struct kvm *kvm, struct vfio_device *vdev, >> /* Ignore invalid or unimplemented regions */ >> if (!region->info.size) >> return 0; >> + if (!is_power_of_two(region->info.size)) { >> + vfio_dev_err(vdev, "region is not power of two: 0x%llx", >> + region->info.size); >> + return -EINVAL; >> + } >> >> if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_MSIX) { >> /* Trap and emulate MSI-X table */ >> diff --git a/virtio/pci.c b/virtio/pci.c >> index 99653cad2c0f..04e801827df9 100644 >> --- a/virtio/pci.c >> +++ b/virtio/pci.c >> @@ -435,6 +435,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev, >> vpci->kvm = kvm; >> vpci->dev = dev; >> >> + BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE)); >> + BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE)); >> + >> r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vdev); >> if (r < 0) >> return r;
On Wed, 15 Jan 2020 12:43:20 +0000 Alexandru Elisei <alexandru.elisei@arm.com> wrote: > Hi, > > On 11/27/19 6:25 PM, Andre Przywara wrote: > > On Mon, 25 Nov 2019 10:30:21 +0000 > > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > > > Hi, > > > >> According to the PCI local bus specification [1], a device's memory size > >> must be a power of two. This is also implicit in the mechanism that a CPU > >> uses to get the memory size requirement for a PCI device. > >> > >> The vesa device requests a memory size that isn't a power of two. > >> According to the same spec [1], a device is allowed to consume more memory > >> than it actually requires. As a result, the amount of memory that the vesa > >> device now reserves has been increased. > >> > >> To prevent slip-ups in the future, a few BUILD_BUG_ON statements were added > >> in places where the memory size is known at compile time. > >> > >> [1] PCI Local Bus Specification Revision 3.0, section 6.2.5.1 > >> > >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > >> --- > >> hw/vesa.c | 2 ++ > >> include/kvm/util.h | 2 ++ > >> include/kvm/vesa.h | 6 +++++- > >> vfio/pci.c | 5 +++++ > >> virtio/pci.c | 3 +++ > >> 5 files changed, 17 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/vesa.c b/hw/vesa.c > >> index f3c5114cf4fe..75670a51be5f 100644 > >> --- a/hw/vesa.c > >> +++ b/hw/vesa.c > >> @@ -58,6 +58,8 @@ struct framebuffer *vesa__init(struct kvm *kvm) > >> char *mem; > >> int r; > >> > >> + BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); > >> + > >> if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) > >> return NULL; > >> > >> diff --git a/include/kvm/util.h b/include/kvm/util.h > >> index 4ca7aa9392b6..e90f1c2db39f 100644 > >> --- a/include/kvm/util.h > >> +++ b/include/kvm/util.h > >> @@ -104,6 +104,8 @@ static inline unsigned long roundup_pow_of_two(unsigned long x) > >> return x ? 1UL << fls_long(x - 1) : 0; > >> } > >> > >> +#define is_power_of_two(x) ((x) ? ((x) & ((x) - 1)) == 0 : 0) > > This gives weird results for negative values (which the kernel avoids by having this a static inline and using an unsigned type). > > Not sure we care, but maybe (x > 0) ? ... would fix this? > > Good point, I will fix it by changing the implicit comparison against 0 at the > beginning with (x) > 0. > > > > >> + > >> struct kvm; > >> void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size); > >> void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size); > >> diff --git a/include/kvm/vesa.h b/include/kvm/vesa.h > >> index 0fac11ab5a9f..e7d971343642 100644 > >> --- a/include/kvm/vesa.h > >> +++ b/include/kvm/vesa.h > >> @@ -5,8 +5,12 @@ > >> #define VESA_HEIGHT 480 > >> > >> #define VESA_MEM_ADDR 0xd0000000 > >> -#define VESA_MEM_SIZE (4*VESA_WIDTH*VESA_HEIGHT) > >> #define VESA_BPP 32 > >> +/* > >> + * We actually only need VESA_BPP/8*VESA_WIDTH*VESA_HEIGHT bytes. But the memory > >> + * size must be a power of 2, so we round up. > >> + */ > >> +#define VESA_MEM_SIZE (1 << 21) > > I don't think it's worth calculating the value and rounding it up, but can we have a BUILD_BUG checking that VESA_MEM_SIZE is big enough to hold the framebuffer? > > I'm not sure what you mean. The current value of VESA_MEM_SIZE is not a power of > two, which breaks the spec. That's the reason for changing it. Are you suggesting > that we keep VESA_MEM_SIZE = 1 << 21, we remove the power_of_two check and add a > BUILD_BUG on VESA_MEM_SIZE < 4 * VESA_WIDTH * VESA_HEIGHT? Only the latter, so keep the power_of_two check, but add the BUILD_BUG with the comparison. So that it breaks if someone increases the width or height. Cheers, Andre. > >> struct kvm; > >> struct biosregs; > >> diff --git a/vfio/pci.c b/vfio/pci.c > >> index 76e24c156906..914732cc6897 100644 > >> --- a/vfio/pci.c > >> +++ b/vfio/pci.c > >> @@ -831,6 +831,11 @@ static int vfio_pci_configure_bar(struct kvm *kvm, struct vfio_device *vdev, > >> /* Ignore invalid or unimplemented regions */ > >> if (!region->info.size) > >> return 0; > >> + if (!is_power_of_two(region->info.size)) { > >> + vfio_dev_err(vdev, "region is not power of two: 0x%llx", > >> + region->info.size); > >> + return -EINVAL; > >> + } > >> > >> if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_MSIX) { > >> /* Trap and emulate MSI-X table */ > >> diff --git a/virtio/pci.c b/virtio/pci.c > >> index 99653cad2c0f..04e801827df9 100644 > >> --- a/virtio/pci.c > >> +++ b/virtio/pci.c > >> @@ -435,6 +435,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev, > >> vpci->kvm = kvm; > >> vpci->dev = dev; > >> > >> + BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE)); > >> + BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE)); > >> + > >> r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vdev); > >> if (r < 0) > >> return r;
Hi, On 1/15/20 2:07 PM, Andre Przywara wrote: > On Wed, 15 Jan 2020 12:43:20 +0000 > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > >> Hi, >> >> On 11/27/19 6:25 PM, Andre Przywara wrote: >>> On Mon, 25 Nov 2019 10:30:21 +0000 >>> Alexandru Elisei <alexandru.elisei@arm.com> wrote: >>> >>> Hi, >>> >>>> According to the PCI local bus specification [1], a device's memory size >>>> must be a power of two. This is also implicit in the mechanism that a CPU >>>> uses to get the memory size requirement for a PCI device. >>>> >>>> The vesa device requests a memory size that isn't a power of two. >>>> According to the same spec [1], a device is allowed to consume more memory >>>> than it actually requires. As a result, the amount of memory that the vesa >>>> device now reserves has been increased. >>>> >>>> To prevent slip-ups in the future, a few BUILD_BUG_ON statements were added >>>> in places where the memory size is known at compile time. >>>> >>>> [1] PCI Local Bus Specification Revision 3.0, section 6.2.5.1 >>>> >>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >>>> --- >>>> hw/vesa.c | 2 ++ >>>> include/kvm/util.h | 2 ++ >>>> include/kvm/vesa.h | 6 +++++- >>>> vfio/pci.c | 5 +++++ >>>> virtio/pci.c | 3 +++ >>>> 5 files changed, 17 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/vesa.c b/hw/vesa.c >>>> index f3c5114cf4fe..75670a51be5f 100644 >>>> --- a/hw/vesa.c >>>> +++ b/hw/vesa.c >>>> @@ -58,6 +58,8 @@ struct framebuffer *vesa__init(struct kvm *kvm) >>>> char *mem; >>>> int r; >>>> >>>> + BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); >>>> + >>>> if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) >>>> return NULL; >>>> >>>> diff --git a/include/kvm/util.h b/include/kvm/util.h >>>> index 4ca7aa9392b6..e90f1c2db39f 100644 >>>> --- a/include/kvm/util.h >>>> +++ b/include/kvm/util.h >>>> @@ -104,6 +104,8 @@ static inline unsigned long roundup_pow_of_two(unsigned long x) >>>> return x ? 1UL << fls_long(x - 1) : 0; >>>> } >>>> >>>> +#define is_power_of_two(x) ((x) ? ((x) & ((x) - 1)) == 0 : 0) >>> This gives weird results for negative values (which the kernel avoids by having this a static inline and using an unsigned type). >>> Not sure we care, but maybe (x > 0) ? ... would fix this? >> Good point, I will fix it by changing the implicit comparison against 0 at the >> beginning with (x) > 0. >> >>> >>>> + >>>> struct kvm; >>>> void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size); >>>> void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size); >>>> diff --git a/include/kvm/vesa.h b/include/kvm/vesa.h >>>> index 0fac11ab5a9f..e7d971343642 100644 >>>> --- a/include/kvm/vesa.h >>>> +++ b/include/kvm/vesa.h >>>> @@ -5,8 +5,12 @@ >>>> #define VESA_HEIGHT 480 >>>> >>>> #define VESA_MEM_ADDR 0xd0000000 >>>> -#define VESA_MEM_SIZE (4*VESA_WIDTH*VESA_HEIGHT) >>>> #define VESA_BPP 32 >>>> +/* >>>> + * We actually only need VESA_BPP/8*VESA_WIDTH*VESA_HEIGHT bytes. But the memory >>>> + * size must be a power of 2, so we round up. >>>> + */ >>>> +#define VESA_MEM_SIZE (1 << 21) >>> I don't think it's worth calculating the value and rounding it up, but can we have a BUILD_BUG checking that VESA_MEM_SIZE is big enough to hold the framebuffer? >> I'm not sure what you mean. The current value of VESA_MEM_SIZE is not a power of >> two, which breaks the spec. That's the reason for changing it. Are you suggesting >> that we keep VESA_MEM_SIZE = 1 << 21, we remove the power_of_two check and add a >> BUILD_BUG on VESA_MEM_SIZE < 4 * VESA_WIDTH * VESA_HEIGHT? > Only the latter, so keep the power_of_two check, but add the BUILD_BUG with the comparison. So that it breaks if someone increases the width or height. Makes sense, I'll do that. Thanks, Alex > > Cheers, > Andre. > >>>> struct kvm; >>>> struct biosregs; >>>> diff --git a/vfio/pci.c b/vfio/pci.c >>>> index 76e24c156906..914732cc6897 100644 >>>> --- a/vfio/pci.c >>>> +++ b/vfio/pci.c >>>> @@ -831,6 +831,11 @@ static int vfio_pci_configure_bar(struct kvm *kvm, struct vfio_device *vdev, >>>> /* Ignore invalid or unimplemented regions */ >>>> if (!region->info.size) >>>> return 0; >>>> + if (!is_power_of_two(region->info.size)) { >>>> + vfio_dev_err(vdev, "region is not power of two: 0x%llx", >>>> + region->info.size); >>>> + return -EINVAL; >>>> + } >>>> >>>> if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_MSIX) { >>>> /* Trap and emulate MSI-X table */ >>>> diff --git a/virtio/pci.c b/virtio/pci.c >>>> index 99653cad2c0f..04e801827df9 100644 >>>> --- a/virtio/pci.c >>>> +++ b/virtio/pci.c >>>> @@ -435,6 +435,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev, >>>> vpci->kvm = kvm; >>>> vpci->dev = dev; >>>> >>>> + BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE)); >>>> + BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE)); >>>> + >>>> r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vdev); >>>> if (r < 0) >>>> return r;
diff --git a/hw/vesa.c b/hw/vesa.c index f3c5114cf4fe..75670a51be5f 100644 --- a/hw/vesa.c +++ b/hw/vesa.c @@ -58,6 +58,8 @@ struct framebuffer *vesa__init(struct kvm *kvm) char *mem; int r; + BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); + if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) return NULL; diff --git a/include/kvm/util.h b/include/kvm/util.h index 4ca7aa9392b6..e90f1c2db39f 100644 --- a/include/kvm/util.h +++ b/include/kvm/util.h @@ -104,6 +104,8 @@ static inline unsigned long roundup_pow_of_two(unsigned long x) return x ? 1UL << fls_long(x - 1) : 0; } +#define is_power_of_two(x) ((x) ? ((x) & ((x) - 1)) == 0 : 0) + struct kvm; void *mmap_hugetlbfs(struct kvm *kvm, const char *htlbfs_path, u64 size); void *mmap_anon_or_hugetlbfs(struct kvm *kvm, const char *hugetlbfs_path, u64 size); diff --git a/include/kvm/vesa.h b/include/kvm/vesa.h index 0fac11ab5a9f..e7d971343642 100644 --- a/include/kvm/vesa.h +++ b/include/kvm/vesa.h @@ -5,8 +5,12 @@ #define VESA_HEIGHT 480 #define VESA_MEM_ADDR 0xd0000000 -#define VESA_MEM_SIZE (4*VESA_WIDTH*VESA_HEIGHT) #define VESA_BPP 32 +/* + * We actually only need VESA_BPP/8*VESA_WIDTH*VESA_HEIGHT bytes. But the memory + * size must be a power of 2, so we round up. + */ +#define VESA_MEM_SIZE (1 << 21) struct kvm; struct biosregs; diff --git a/vfio/pci.c b/vfio/pci.c index 76e24c156906..914732cc6897 100644 --- a/vfio/pci.c +++ b/vfio/pci.c @@ -831,6 +831,11 @@ static int vfio_pci_configure_bar(struct kvm *kvm, struct vfio_device *vdev, /* Ignore invalid or unimplemented regions */ if (!region->info.size) return 0; + if (!is_power_of_two(region->info.size)) { + vfio_dev_err(vdev, "region is not power of two: 0x%llx", + region->info.size); + return -EINVAL; + } if (pdev->irq_modes & VFIO_PCI_IRQ_MODE_MSIX) { /* Trap and emulate MSI-X table */ diff --git a/virtio/pci.c b/virtio/pci.c index 99653cad2c0f..04e801827df9 100644 --- a/virtio/pci.c +++ b/virtio/pci.c @@ -435,6 +435,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev, vpci->kvm = kvm; vpci->dev = dev; + BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE)); + BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE)); + r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vdev); if (r < 0) return r;
According to the PCI local bus specification [1], a device's memory size must be a power of two. This is also implicit in the mechanism that a CPU uses to get the memory size requirement for a PCI device. The vesa device requests a memory size that isn't a power of two. According to the same spec [1], a device is allowed to consume more memory than it actually requires. As a result, the amount of memory that the vesa device now reserves has been increased. To prevent slip-ups in the future, a few BUILD_BUG_ON statements were added in places where the memory size is known at compile time. [1] PCI Local Bus Specification Revision 3.0, section 6.2.5.1 Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- hw/vesa.c | 2 ++ include/kvm/util.h | 2 ++ include/kvm/vesa.h | 6 +++++- vfio/pci.c | 5 +++++ virtio/pci.c | 3 +++ 5 files changed, 17 insertions(+), 1 deletion(-)