| Message ID | 20200326152438.6218-19-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add reassignable BARs and PCIE 1.1 support | expand |
On 26/03/2020 15:24, Alexandru Elisei wrote: > If we try to register a range of ports which overlaps with another, already > registered, I/O ports region then device emulation for that region will not > work anymore. There's nothing sane that the ioport emulation layer can do > in this case so refuse to allocate the port. This matches the behavior of > kvm__register_mmio. > > There's no need to protect allocating a new ioport struct with a lock, so > move the lock to protect the actual ioport insertion in the tree. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> That looks correct to me. The protection of the ioport_tree by the br_lock would deserve some mentioning in a comment, but this is a separate issue, so: Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > ioport.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/ioport.c b/ioport.c > index cb778ed8d757..d9f2e8ea3c3b 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -68,14 +68,6 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i > struct ioport *entry; > int r; > > - br_write_lock(kvm); > - > - entry = ioport_search(&ioport_tree, port); > - if (entry) { > - pr_warning("ioport re-registered: %x", port); > - ioport_remove(&ioport_tree, entry); > - } > - > entry = malloc(sizeof(*entry)); > if (entry == NULL) > return -ENOMEM; > @@ -90,6 +82,7 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i > }, > }; > > + br_write_lock(kvm); > r = ioport_insert(&ioport_tree, entry); > if (r < 0) > goto out_free; >
diff --git a/ioport.c b/ioport.c index cb778ed8d757..d9f2e8ea3c3b 100644 --- a/ioport.c +++ b/ioport.c @@ -68,14 +68,6 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i struct ioport *entry; int r; - br_write_lock(kvm); - - entry = ioport_search(&ioport_tree, port); - if (entry) { - pr_warning("ioport re-registered: %x", port); - ioport_remove(&ioport_tree, entry); - } - entry = malloc(sizeof(*entry)); if (entry == NULL) return -ENOMEM; @@ -90,6 +82,7 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i }, }; + br_write_lock(kvm); r = ioport_insert(&ioport_tree, entry); if (r < 0) goto out_free;
If we try to register a range of ports which overlaps with another, already registered, I/O ports region then device emulation for that region will not work anymore. There's nothing sane that the ioport emulation layer can do in this case so refuse to allocate the port. This matches the behavior of kvm__register_mmio. There's no need to protect allocating a new ioport struct with a lock, so move the lock to protect the actual ioport insertion in the tree. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- ioport.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)