Re: [PATCH 11/18] hw/nvram/fw_cfg: Remove FWCfgState::file_slots field

[Igor Mammedov <imammedo@redhat.com>]

On Thu,  1 May 2025 23:04:49 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> The FWCfgState::file_slots integer was only set in the
> hw_compat_2_8[] array, via the 'x-file-slots=0x10' property.
> We removed all machines using that array, lets remove that
> property and all the code around it. The number of slots is
> always FW_CFG_FILE_SLOTS_DFLT (32).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I'd keep this one,
alternatively if we decide to nuke it in favor of a constant,
I'd get rid of fw_cfg_file_slots() and simplify fw_cfg_file_slots_allocate() at the same time

Gerd,
 WDYT?

> ---
>  include/hw/nvram/fw_cfg.h |  1 -
>  hw/nvram/fw_cfg.c         | 24 +-----------------------
>  2 files changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index d41b9328fd1..07a31d214db 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -59,7 +59,6 @@ struct FWCfgState {
>      SysBusDevice parent_obj;
>      /*< public >*/
>  
> -    uint16_t file_slots;
>      FWCfgEntry *entries[2];
>      int *entry_order;
>      FWCfgFiles *files;
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 54cfa07d3f5..71c41c979d7 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -257,7 +257,7 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value)
>  
>  static inline uint16_t fw_cfg_file_slots(const FWCfgState *s)
>  {
> -    return s->file_slots;
> +    return FW_CFG_FILE_SLOTS_DFLT;
>  }
>
>  
>  /* Note: this function returns an exclusive limit. */
> @@ -1153,24 +1153,6 @@ static const TypeInfo fw_cfg_info = {
>  
>  static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
>  {
> -    uint16_t file_slots_max;
> -
> -    if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) {
> -        error_setg(errp, "\"file_slots\" must be at least 0x%x",
> -                   FW_CFG_FILE_SLOTS_MIN);
> -        return;
> -    }
> -
> -    /* (UINT16_MAX & FW_CFG_ENTRY_MASK) is the highest inclusive selector value
> -     * that we permit. The actual (exclusive) value coming from the
> -     * configuration is (FW_CFG_FILE_FIRST + fw_cfg_file_slots(s)). */
> -    file_slots_max = (UINT16_MAX & FW_CFG_ENTRY_MASK) - FW_CFG_FILE_FIRST + 1;
> -    if (fw_cfg_file_slots(s) > file_slots_max) {
> -        error_setg(errp, "\"file_slots\" must not exceed 0x%" PRIx16,
> -                   file_slots_max);
> -        return;
> -    }
> -
>      s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
>      s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
>      s->entry_order = g_new0(int, fw_cfg_max_entry(s));
> @@ -1179,8 +1161,6 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
>  static const Property fw_cfg_io_properties[] = {
>      DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
>                       true),
> -    DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
> -                       FW_CFG_FILE_SLOTS_DFLT),
>  };
>  
>  static void fw_cfg_io_realize(DeviceState *dev, Error **errp)
> @@ -1228,8 +1208,6 @@ static const Property fw_cfg_mem_properties[] = {
>      DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
>      DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
>                       true),
> -    DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots,
> -                       FW_CFG_FILE_SLOTS_DFLT),
>  };
>  
>  static void fw_cfg_mem_realize(DeviceState *dev, Error **errp)