Re: [Qemu-devel] [PING][PATCH] Added the -m flag feature to stellaris boards

[Peter Maydell <peter.maydell@linaro.org>]

On 21 March 2016 at 18:42, Aurelio Remonda
<aurelio.remonda@tallertechnologies.com> wrote:
> This patch adds the memory flag to both stellaris LM3S811EVB
> and LM3S6965EVB.
>
> The hardcoded dc0 values for both boards still exists but now the higher 16 bits
> are calculated based on ram_size which could be either user-given or the default
> one. Then the sram_size is calculated as usual, flash_size is not affected by
> this.
>
> As the higher part of dc0 has a top value of 0xffff, the boards won't accept a
> size value larger than 16M. We'll throw an error if the user tries to make the
> RAM larger than that.
>
> The default RAM sizes are now set in the boards' respective class_init
> functions.
>
> I tested this on the LM3S6965evb doing a full system emulation.
> I couldn't try this on the LM3S811EVB since I am using RTEMS and it does
> not support that board.
>
> Signed-off-by: Aurelio Remonda <aurelio.remonda@tallertechnologies.com>
>
> ---
>  hw/arm/stellaris.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 0114e0a..dbbb1c8 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -17,6 +17,7 @@
>  #include "hw/boards.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/error-report.h"
>
>  #define GPIO_A 0
>  #define GPIO_B 1
> @@ -31,17 +32,21 @@
>  #define BP_GAMEPAD   0x04
>
>  #define NUM_IRQ_LINES 64
> +#define LM3S811EVB_DEFAULT_DC0 0x00001f00 /* Default value for dc0 sram_size half */
> +#define LM3S6965EVB_DEFAULT_DC0 0x0000ff00 /* Default value for dc0 sram_size half */

These don't seem to be the same as the default values we had previously ?

> +#define DC0_MAX_SRAM 0xffff /* Maximum value for sram half in dc0 register */
> +#define DC0_SRAM_SHIFT 16
>
> -typedef const struct {
> +typedef struct {
>      const char *name;
> -    uint32_t did0;
> -    uint32_t did1;
> +    const uint32_t did0;
> +    const uint32_t did1;
>      uint32_t dc0;
> -    uint32_t dc1;
> -    uint32_t dc2;
> -    uint32_t dc3;
> -    uint32_t dc4;
> -    uint32_t peripherals;
> +    const uint32_t dc1;
> +    const uint32_t dc2;
> +    const uint32_t dc3;
> +    const uint32_t dc4;
> +    const uint32_t peripherals;
>  } stellaris_board_info;
>
>  /* General purpose timer module.  */
> @@ -1190,7 +1195,7 @@ static stellaris_board_info stellaris_boards[] = {
>    { "LM3S811EVB",
>      0,
>      0x0032000e,
> -    0x001f001f, /* dc0 */
> +    0x0000001f, /* dc0 */
>      0x001132bf,
>      0x01071013,
>      0x3f0f01ff,
> @@ -1200,7 +1205,7 @@ static stellaris_board_info stellaris_boards[] = {
>    { "LM3S6965EVB",
>      0x10010002,
>      0x1073402e,
> -    0x00ff007f, /* dc0 */
> +    0x0000007f, /* dc0 */
>      0x001133ff,
>      0x030f5317,
>      0x0f0f87ff,
> @@ -1223,7 +1228,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>      qemu_irq gpio_in[7][8];
>      qemu_irq gpio_out[7][8];
>      qemu_irq adc;
> -    int sram_size;
> +    unsigned int sram_size;
>      int flash_size;
>      I2CBus *i2c;
>      DeviceState *dev;
> @@ -1234,6 +1239,15 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>      MemoryRegion *flash = g_new(MemoryRegion, 1);
>      MemoryRegion *system_memory = get_system_memory();
>
> +    /* RAM size should be divided by 256 in order to get a valid 16 bits dc0 value */
> +    ram_size = (ram_size >> 8) - 1;
> +
> +    if (ram_size > DC0_MAX_SRAM) {
> +        error_report("Requested RAM size is too big for this board. The maximum allowed is 16M.");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    board->dc0 |= ram_size << DC0_SRAM_SHIFT;
>      flash_size = (((board->dc0 & 0xffff) + 1) << 1) * 1024;
>      sram_size = ((board->dc0 >> 18) + 1) * 1024;

Do you know why your DC0_SRAM_SHIFT is 16 but this line which
calculates sram_size from board->dc0 is doing a shift by 18 ?

> @@ -1391,6 +1405,7 @@ static void lm3s811evb_class_init(ObjectClass *oc, void *data)
>
>      mc->desc = "Stellaris LM3S811EVB";
>      mc->init = lm3s811evb_init;
> +    mc->default_ram_size = LM3S811EVB_DEFAULT_DC0;

The default_ram_size should be a size in bytes, not a DC0 value.

>  }
>
>  static const TypeInfo lm3s811evb_type = {
> @@ -1405,6 +1420,7 @@ static void lm3s6965evb_class_init(ObjectClass *oc, void *data)
>
>      mc->desc = "Stellaris LM3S6965EVB";
>      mc->init = lm3s6965evb_init;
> +    mc->default_ram_size = LM3S6965EVB_DEFAULT_DC0;
>  }
>
>  static const TypeInfo lm3s6965evb_type = {
> --
> 2.7.3

thanks
-- PMM