From: Cédric Le Goater <clg@kaod.org>

Recent firmwares uses SPI DMA transfers in U-Boot to load the

different images (kernel, initrd, dtb) in the SoC DRAM. The AST2600

FMC model is missing the masks to be applied on the DMA registers

which resulted in incorrect values. Fix that and wire the SPI

controllers which have DMA support on the AST2600.

Fixes: bcaa8ddd081c ("aspeed/smc: Add AST2600 support")

Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Joel Stanley <joel@jms.id.au>

Message-id: 20200320053923.20565-1-clg@kaod.org

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

hw/arm/aspeed_ast2600.c | 6 ++++++

hw/ssi/aspeed_smc.c | 15 +++++++++++++--

hw/ssi/trace-events | 1 +

3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/aspeed_ast2600.c

+++ b/hw/arm/aspeed_ast2600.c

@@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)

/* SPI */

for (i = 0; i < sc->spis_num; i++) {

+ object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),

+ "dram", &err);

+ if (err) {

+ error_propagate(errp, err);

+ return;

+ }

object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);

object_property_set_bool(OBJECT(&s->spi[i]), true, "realized",

&local_err);

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/ssi/aspeed_smc.c

+++ b/hw/ssi/aspeed_smc.c

@@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = {

.flash_window_base = ASPEED26_SOC_FMC_FLASH_BASE,

.flash_window_size = 0x10000000,

.has_dma = true,

+ .dma_flash_mask = 0x0FFFFFFC,

+ .dma_dram_mask = 0x3FFFFFFC,

.nregs = ASPEED_SMC_R_MAX,

.segment_to_reg = aspeed_2600_smc_segment_to_reg,

.reg_to_segment = aspeed_2600_smc_reg_to_segment,

@@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = {

.segments = aspeed_segments_ast2600_spi1,

.flash_window_base = ASPEED26_SOC_SPI_FLASH_BASE,

.flash_window_size = 0x10000000,

- .has_dma = false,

+ .has_dma = true,

+ .dma_flash_mask = 0x0FFFFFFC,

+ .dma_dram_mask = 0x3FFFFFFC,

.nregs = ASPEED_SMC_R_MAX,

.segment_to_reg = aspeed_2600_smc_segment_to_reg,

.reg_to_segment = aspeed_2600_smc_reg_to_segment,

@@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = {

.segments = aspeed_segments_ast2600_spi2,

.flash_window_base = ASPEED26_SOC_SPI2_FLASH_BASE,

.flash_window_size = 0x10000000,

- .has_dma = false,

+ .has_dma = true,

+ .dma_flash_mask = 0x0FFFFFFC,

+ .dma_dram_mask = 0x3FFFFFFC,

.nregs = ASPEED_SMC_R_MAX,

.segment_to_reg = aspeed_2600_smc_segment_to_reg,

.reg_to_segment = aspeed_2600_smc_reg_to_segment,

@@ -XXX,XX +XXX,XX @@ static void aspeed_smc_dma_rw(AspeedSMCState *s)

MemTxResult result;

uint32_t data;

+ trace_aspeed_smc_dma_rw(s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE ?

+ "write" : "read",

+ s->regs[R_DMA_FLASH_ADDR],

+ s->regs[R_DMA_DRAM_ADDR],

+ s->regs[R_DMA_LEN]);

while (s->regs[R_DMA_LEN]) {

if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {

data = address_space_ldl_le(&s->dram_as, s->regs[R_DMA_DRAM_ADDR],

diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/hw/ssi/trace-events

+++ b/hw/ssi/trace-events

@@ -XXX,XX +XXX,XX @@ aspeed_smc_do_snoop(int cs, int index, int dummies, int data) "CS%d index:0x%x d

aspeed_smc_flash_write(int cs, uint64_t addr, uint32_t size, uint64_t data, int mode) "CS%d @0x%" PRIx64 " size %u: 0x%" PRIx64" mode:%d"

aspeed_smc_read(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64

aspeed_smc_dma_checksum(uint32_t addr, uint32_t data) "0x%08x: 0x%08x"

+aspeed_smc_dma_rw(const char *dir, uint32_t flash_addr, uint32_t dram_addr, uint32_t size) "%s flash:@0x%08x dram:@0x%08x size:0x%08x"

aspeed_smc_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size %u: 0x%" PRIx64

aspeed_smc_flash_select(int cs, const char *prefix) "CS%d %sselect"

--

2.20.1

From: Richard Henderson <richard.henderson@linaro.org>

Coverity rightly notes that ctz32(bas) on 0 will return 32,

which makes the len calculation a BAD_SHIFT.

A value of 0 in DBGWCR<n>_EL1.BAS is reserved. Simply move

the existing check we have for this case.

Reported-by: Coverity (CID 1421964)

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20200320160622.8040-2-richard.henderson@linaro.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/helper.c | 11 ++++++-----

1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ void hw_watchpoint_update(ARMCPU *cpu, int n)

int bas = extract64(wcr, 5, 8);

int basstart;

- if (bas == 0) {

- /* This must act as if the watchpoint is disabled */

- return;

- }

-

if (extract64(wvr, 2, 1)) {

/* Deprecated case of an only 4-aligned address. BAS[7:4] are

* ignored, and BAS[3:0] define which bytes to watch.

*/

bas &= 0xf;

}

+

+ if (bas == 0) {

+ /* This must act as if the watchpoint is disabled */

+ return;

+ }

+

/* The BAS bits are supposed to be programmed to indicate a contiguous

* range of bytes. Otherwise it is CONSTRAINED UNPREDICTABLE whether

* we fire for each byte in the word/doubleword addressed by the WVR.

--

2.20.1

From: Richard Henderson <richard.henderson@linaro.org>

Coverity raised a shed-load of errors cascading from inferring

that clz32(immh) might yield 32, from immh might be 0.

While immh cannot be 0 from encoding, it is not obvious even to

a human how we've checked that: via the filtering provided by

data_proc_simd[].

Reported-by: Coverity (CID 1421923, and more)

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20200320160622.8040-3-richard.henderson@linaro.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate-a64.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/translate-a64.c

+++ b/target/arm/translate-a64.c

@@ -XXX,XX +XXX,XX @@ static void disas_simd_shift_imm(DisasContext *s, uint32_t insn)

bool is_u = extract32(insn, 29, 1);

bool is_q = extract32(insn, 30, 1);

+ /* data_proc_simd[] has sent immh == 0 to disas_simd_mod_imm. */

+ assert(immh != 0);

+

switch (opcode) {

case 0x08: /* SRI */

if (!is_u) {

--

2.20.1

From: Richard Henderson <richard.henderson@linaro.org>

Coverity reports a BAD_SHIFT with ctz32(imm5), with imm5 == 0.

This is an invalid encoding, but we diagnose that just below

by rejecting size > 3. Avoid the warning by sinking the

computation of index below the check.

Reported-by: Coverity (CID 1421965)

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20200320160622.8040-4-richard.henderson@linaro.org

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate-a64.c | 3 ++-

1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/translate-a64.c

+++ b/target/arm/translate-a64.c

@@ -XXX,XX +XXX,XX @@ static void handle_simd_dupe(DisasContext *s, int is_q, int rd, int rn,

int imm5)

{

int size = ctz32(imm5);

- int index = imm5 >> (size + 1);

+ int index;

if (size > 3 || (size == 3 && !is_q)) {

unallocated_encoding(s);

@@ -XXX,XX +XXX,XX @@ static void handle_simd_dupe(DisasContext *s, int is_q, int rd, int rn,

return;

}

+ index = imm5 >> (size + 1);

tcg_gen_gvec_dup_mem(size, vec_full_reg_offset(s, rd),

vec_reg_offset(s, rn, index, size),

is_q ? 16 : 8, vec_full_reg_size(s));

--

2.20.1