[PATCH qemu 21/38] aspeed/smc: adjust the size of the register region

[Andrew Jeffery]

On Fri, 2016-11-18 at 15:22 +0100, Cédric Le Goater wrote:
> The SPI controller of the AST2400 SoC does not support DMAs and
> segment registers, hence it has less registers. So we can adjust the
> size of the memory region holding the registers depending on the
> controller type. We can also remove the guest_error logging which is
> useless as the range of the region is strict enough.
> 
> > Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
>  hw/ssi/aspeed_smc.c         | 23 ++++++++---------------
>  include/hw/ssi/aspeed_smc.h |  1 +
>  2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index c0ae07ef8c96..7d7800d96271 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -135,6 +135,8 @@
>  #define R_SPI_MISC_CTRL   (0x10 / 4)
>  #define R_SPI_TIMINGS     (0x14 / 4)
>  
> +#define R_SPI_MAX         (0x20 / 4)
> +
>  #define ASPEED_SOC_SMC_FLASH_BASE   0x10000000
>  #define ASPEED_SOC_FMC_FLASH_BASE   0x20000000
>  #define ASPEED_SOC_SPI_FLASH_BASE   0x30000000
> @@ -221,6 +223,7 @@ static const AspeedSMCController controllers[] = {
>          .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE,
>          .flash_window_size = 0x10000000,
>          .has_dma           = true,
> +        .nregs             = ASPEED_SMC_R_MAX,
>      }, {
>          .name              = "aspeed.smc.spi",
>          .r_conf            = R_SPI_CONF,
> @@ -233,6 +236,7 @@ static const AspeedSMCController controllers[] = {
>          .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE,
>          .flash_window_size = 0x10000000,
>          .has_dma           = false,
> +        .nregs             = R_SPI_MAX,
>      }, {
>          .name              = "aspeed.smc.ast2500-fmc",
>          .r_conf            = R_CONF,
> @@ -245,6 +249,7 @@ static const AspeedSMCController controllers[] = {
>          .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE,
>          .flash_window_size = 0x10000000,
>          .has_dma           = true,
> +        .nregs             = ASPEED_SMC_R_MAX,
>      }, {
>          .name              = "aspeed.smc.ast2500-spi1",
>          .r_conf            = R_CONF,
> @@ -257,6 +262,7 @@ static const AspeedSMCController controllers[] = {
>          .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE,
>          .flash_window_size = 0x8000000,
>          .has_dma           = false,
> +        .nregs             = ASPEED_SMC_R_MAX,
>      }, {
>          .name              = "aspeed.smc.ast2500-spi2",
>          .r_conf            = R_CONF,
> @@ -269,6 +275,7 @@ static const AspeedSMCController controllers[] = {
>          .flash_window_base = ASPEED_SOC_SPI2_FLASH_BASE,
>          .flash_window_size = 0x8000000,
>          .has_dma           = false,
> +        .nregs             = ASPEED_SMC_R_MAX,
>      },
>  };
>  
> @@ -712,13 +719,6 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
>  
>      addr >>= 2;
>  
> -    if (addr >= ARRAY_SIZE(s->regs)) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                      "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n",
> -                      __func__, addr);
> -        return 0;
> -    }
> -
>      if (addr == s->r_conf ||
>          addr == s->r_timings ||
>          addr == s->r_ce_ctrl ||
> @@ -942,13 +942,6 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data,
>  
>      addr >>= 2;
>  
> -    if (addr >= ARRAY_SIZE(s->regs)) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                      "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n",
> -                      __func__, addr);
> -        return;
> -    }
> -
>      if (addr == s->r_conf ||
>          addr == s->r_timings ||
>          addr == s->r_ce_ctrl) {
> @@ -1030,7 +1023,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>  
>      /* The memory region for the controller registers */
>      memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s,
> -                          s->ctrl->name, ASPEED_SMC_R_MAX * 4);
> +                          s->ctrl->name, s->ctrl->nregs * 4);

It looks like you missed defining nregs for the "aspeed.smc.smc"
controller. This would suggest we need to.

Andrew

>      sysbus_init_mmio(sbd, &s->mmio);
>  
>      /*
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 3ae0a369073d..91bad82e9c65 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -45,6 +45,7 @@ typedef struct AspeedSMCController {
>      hwaddr flash_window_base;
>      uint32_t flash_window_size;
>      bool has_dma;
> +    uint32_t nregs;
>  } AspeedSMCController;
>  
>  typedef struct AspeedSMCFlash {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20161128/69f13b39/attachment.sig>