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@xxxxxxx>
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@xxxxxxx>
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;
>
