Re: [PATCH v2 04/20] nubus: use bitmap to manage available slots

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 9/15/21 12:10 PM, Mark Cave-Ayland wrote:
> Convert nubus_device_realize() to use a bitmap to manage available slots to allow
> for future Nubus devices to be plugged into arbitrary slots from the command line.
> 
> Update mac_nubus_bridge_init() to only allow slots 0x9 to 0xe on a Macintosh
> machines as documented in "Desigining Cards and Drivers for the Macintosh Family".
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/nubus/mac-nubus-bridge.c |  3 +++
>  hw/nubus/nubus-bus.c        |  2 +-
>  hw/nubus/nubus-device.c     | 33 +++++++++++++++++++++++++++------
>  include/hw/nubus/nubus.h    |  4 ++--
>  4 files changed, 33 insertions(+), 9 deletions(-)

>  static void nubus_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
> index c1832f73da..f9f614cc01 100644
> --- a/hw/nubus/nubus-device.c
> +++ b/hw/nubus/nubus-device.c
> @@ -160,14 +160,35 @@ static void nubus_device_realize(DeviceState *dev, Error **errp)
>      NubusDevice *nd = NUBUS_DEVICE(dev);
>      char *name;
>      hwaddr slot_offset;
> -
> -    if (nubus->current_slot < NUBUS_FIRST_SLOT ||
> -            nubus->current_slot > NUBUS_LAST_SLOT) {
> -        error_setg(errp, "Cannot register nubus card, not enough slots");
> -        return;
> +    uint16_t s;
> +
> +    if (nd->slot == -1) {
> +        /* No slot specified, find first available free slot */
> +        s = ctz32(nubus->slot_available_mask);
> +        if (s != 32) {
> +            nd->slot = s;

Can't we have s >= NUBUS_LAST_SLOT here?

> +        } else {
> +            error_setg(errp, "Cannot register nubus card, no free slot "
> +                             "available");
> +            return;
> +        }
> +    } else {

If so, maybe better to move this code <...

> +        /* Slot specified, make sure the slot is available */
> +        if (nd->slot < NUBUS_FIRST_SLOT || nd->slot > NUBUS_LAST_SLOT) {
> +            error_setg(errp, "Cannot register nubus card, slot must be "
> +                             "between %d and %d", NUBUS_FIRST_SLOT,
> +                             NUBUS_LAST_SLOT);
> +            return;
> +        }

...> ...

> +
> +        if (!(nubus->slot_available_mask & (1UL << nd->slot))) {
> +            error_setg(errp, "Cannot register nubus card, slot %d is "
> +                             "unavailable or already occupied", nd->slot);
> +            return;
> +        }
>      }

... here?

>  
> -    nd->slot = nubus->current_slot++;
> +    nubus->slot_available_mask &= ~(1UL << nd->slot);
>  
>      /* Super */
>      slot_offset = (nd->slot - 6) * NUBUS_SUPER_SLOT_SIZE;
> diff --git a/include/hw/nubus/nubus.h b/include/hw/nubus/nubus.h
> index 357f621d15..8ff4736259 100644
> --- a/include/hw/nubus/nubus.h
> +++ b/include/hw/nubus/nubus.h
> @@ -19,7 +19,7 @@
>  #define NUBUS_SLOT_SIZE       0x01000000
>  #define NUBUS_SLOT_NB         0xF
>  
> -#define NUBUS_FIRST_SLOT      0x9
> +#define NUBUS_FIRST_SLOT      0x0
>  #define NUBUS_LAST_SLOT       0xF
>  
>  #define TYPE_NUBUS_DEVICE "nubus-device"
> @@ -36,7 +36,7 @@ struct NubusBus {
>      MemoryRegion super_slot_io;
>      MemoryRegion slot_io;
>  
> -    int current_slot;
> +    uint32_t slot_available_mask;
>  };
>  
>  struct NubusDevice {
>