RE: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

2024-05-14 Thread Jamin Lin
Hi Cedric,

Sorry reply you late.
> On 4/30/24 10:51, Jamin Lin wrote:
> > Hi Cedric,
> >> On 4/19/24 15:41, Cédric Le Goater wrote:
> >>> On 4/16/24 11:18, Jamin Lin wrote:
>  DMA length is from 1 byte to 32MB for AST2600 and AST10x0 and DMA
>  length is from 4 bytes to 32MB for AST2500.
> 
>  In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte
>  data for AST2600 and AST10x0 and 4 bytes data for AST2500.
> > To support all ASPEED SOCs, adds dma_start_length parameter to
> > store
>  the start length, add helper routines function to compute the dma
>  length and update DMA_LENGTH mask to "1FF" to fix dma moving
>  incorrect data length issue.
> >>>
> >>> OK. There are two problems to address, the "zero" length transfer
> >>> and the DMA length unit, which is missing today. Newer SoC use a 1
> >>> bit / byte and older ones, AST2400 and AST2500, use 1 bit / 4 bytes.
> >>>
> >>> We can introduce a AspeedSMCClass::dma_len_unit and rework the loop
> to :
> >>>
> >>>       do {
> >>>
> >>>     
> >>>
> >>>      if (s->regs[R_DMA_LEN]) {
> >>>       s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit;
> >>>       }
> >>>       } while (s->regs[R_DMA_LEN]);
> >>>
> >>> It should fix the current implementation.
> >>
> >>
> >> I checked what FW is doing on a QEMU ast2500-evb machine :
> >>
> >>   U-Boot 2019.04-v00.04.12 (Sep 29 2022 - 10:40:37 +)
> >>   ...
> >>
> >>  Loading Kernel Image ... aspeed_smc_write @0x88 size 4:
> >> 0x80001000
> >>   aspeed_smc_write @0x84 size 4: 0x20100130
> >>   aspeed_smc_write @0x8c size 4: 0x3c6770
> >>   aspeed_smc_write @0x80 size 4: 0x1
> >>   aspeed_smc_dma_rw read flash:@0x00100130 dram:@0x1000
> >> size:0x003c6774
> >>   aspeed_smc_read @0x8 size 4: 0x800
> >>   aspeed_smc_write @0x80 size 4: 0x0
> >>   OK
> >>  Loading Ramdisk to 8fef2000, end 8604 ...
> >> aspeed_smc_write
> >> @0x88 size 4: 0x8fef2000
> >>   aspeed_smc_write @0x84 size 4: 0x204cdde0
> >>   aspeed_smc_write @0x8c size 4: 0x10d604
> >>   aspeed_smc_write @0x80 size 4: 0x1
> >>   aspeed_smc_dma_rw read flash:@0x004cdde0 dram:@0xfef2000
> >> size:0x0010d608
> >>   aspeed_smc_read @0x8 size 4: 0x800
> >>   aspeed_smc_write @0x80 size 4: 0x0
> >>   OK
> >>  Loading Device Tree to 8fee7000, end 8fef135e ...
> >> aspeed_smc_write
> >> @0x88 size 4: 0x8fee7000
> >>   aspeed_smc_write @0x84 size 4: 0x204c69b4
> >>   aspeed_smc_write @0x8c size 4: 0x7360
> >>   aspeed_smc_write @0x80 size 4: 0x1
> >>   aspeed_smc_dma_rw read flash:@0x004c69b4 dram:@0xfee7000
> >> size:0x7364
> >>   aspeed_smc_read @0x8 size 4: 0x800
> >>   aspeed_smc_write @0x80 size 4: 0x0
> >>   OK
> >>
> >>   Starting kernel ...
> >>
> >> It seems that the R_DMA_LEN register is set by FW without taking into
> >> account the length unit ( bit / 4 bytes). Would you know why ?
> >>
After I discussed with designers, my explanation as below.
AST2500 datasheet description:
FMC8C: DMA Length Register
31:25  Reserved(0)
24:2  RW  DMA_LENGTH
  From 4bytes to 32MB
  0: 4bytes
  0x7F: 32MB
1:0   Reserved

If users set this register 0, SPI DMA controller would move 4 bytes data.
If users set this register bits[24:2] 0x7F, SPI DMC controller would move 
32MB data because this register includes reserved bits [1:0].
Ex: bits 24:2 --> 0x7f
   bits 1:0 --> 0 Reserved  
bits[24:0] is (0x1FC + start length 4) --> 200 --> 32MB  
   111       00
LENGTH[24:2] 7   FF   F   FFReserved [1:0]

That was why AST2500 DMA length should 4 bytes alignment and you do not see 
handling length units because it includes reserved bits 1 and 0.
If user want to move 3 bytes data, our firmware set this register 4 ensure it 
4bytes alignment and the value of register as following
bits[2:0] = "b100" Finally, SPI DMA controller move 8 bytes data(4bytes input 
count + start length 4bytes) 

And that was why it set DMA_LENGTH 0x01FC
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L187
#define DMA_LENGTH(val) ((val) & 0x01FC)

I want to change it to 0x01FF to support all ASPEED SOCs because AST2600, 
AST2700 and AST1030 use this register bit 0 and 1 whose unit is 1byte rather 
than 4 bytes.

> > https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/l
> > ib/string.c#L559 This line make user input data length 4 bytes
> > alignment.
> >
> https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/a
> > rch/arm/mach-aspeed/ast2500/utils.S#L35
> 
> yes. I don't see any 1bit / 4bytes conversion when setting the DMA_LEN
> register. Am I mistaking ? That's not what the specs says.
> 
> > This line set the value of count parameter to AST_FMC_DNA_LENGTH.
> > AST_FMC_DMA_LENGTH is 4 bytes alignment value.
> > Example: input 4
> > ((4+3)/4) * 4 --> 

Re: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

2024-05-03 Thread Cédric Le Goater

On 4/30/24 10:51, Jamin Lin wrote:

Hi Cedric,

On 4/19/24 15:41, Cédric Le Goater wrote:

On 4/16/24 11:18, Jamin Lin wrote:

DMA length is from 1 byte to 32MB for AST2600 and AST10x0 and DMA
length is from 4 bytes to 32MB for AST2500.

In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte
data for AST2600 and AST10x0 and 4 bytes data for AST2500.

To support all ASPEED SOCs, adds dma_start_length parameter to store

the start length, add helper routines function to compute the dma
length and update DMA_LENGTH mask to "1FF" to fix dma moving
incorrect data length issue.


OK. There are two problems to address, the "zero" length transfer and
the DMA length unit, which is missing today. Newer SoC use a 1 bit /
byte and older ones, AST2400 and AST2500, use 1 bit / 4 bytes.

We can introduce a AspeedSMCClass::dma_len_unit and rework the loop to :

      do {

    

     if (s->regs[R_DMA_LEN]) {
      s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit;
      }
      } while (s->regs[R_DMA_LEN]);

It should fix the current implementation.



I checked what FW is doing on a QEMU ast2500-evb machine :

  U-Boot 2019.04-v00.04.12 (Sep 29 2022 - 10:40:37 +)
  ...

 Loading Kernel Image ... aspeed_smc_write @0x88 size 4:
0x80001000
  aspeed_smc_write @0x84 size 4: 0x20100130
  aspeed_smc_write @0x8c size 4: 0x3c6770
  aspeed_smc_write @0x80 size 4: 0x1
  aspeed_smc_dma_rw read flash:@0x00100130 dram:@0x1000
size:0x003c6774
  aspeed_smc_read @0x8 size 4: 0x800
  aspeed_smc_write @0x80 size 4: 0x0
  OK
 Loading Ramdisk to 8fef2000, end 8604 ... aspeed_smc_write
@0x88 size 4: 0x8fef2000
  aspeed_smc_write @0x84 size 4: 0x204cdde0
  aspeed_smc_write @0x8c size 4: 0x10d604
  aspeed_smc_write @0x80 size 4: 0x1
  aspeed_smc_dma_rw read flash:@0x004cdde0 dram:@0xfef2000
size:0x0010d608
  aspeed_smc_read @0x8 size 4: 0x800
  aspeed_smc_write @0x80 size 4: 0x0
  OK
 Loading Device Tree to 8fee7000, end 8fef135e ... aspeed_smc_write
@0x88 size 4: 0x8fee7000
  aspeed_smc_write @0x84 size 4: 0x204c69b4
  aspeed_smc_write @0x8c size 4: 0x7360
  aspeed_smc_write @0x80 size 4: 0x1
  aspeed_smc_dma_rw read flash:@0x004c69b4 dram:@0xfee7000
size:0x7364
  aspeed_smc_read @0x8 size 4: 0x800
  aspeed_smc_write @0x80 size 4: 0x0
  OK

  Starting kernel ...

It seems that the R_DMA_LEN register is set by FW without taking into account
the length unit ( bit / 4 bytes). Would you know why ?


https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/lib/string.c#L559
This line make user input data length 4 bytes alignment.
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/arch/arm/mach-aspeed/ast2500/utils.S#L35


yes. I don't see any 1bit / 4bytes conversion when setting the DMA_LEN
register. Am I mistaking ? That's not what the specs says.


This line set the value of count parameter to AST_FMC_DNA_LENGTH.
AST_FMC_DMA_LENGTH is 4 bytes alignment value.
Example: input 4
((4+3)/4) * 4 --> (7/4) *4 ---> 4
If AST_FMC_DMA_LENGTH is 0, it means it should move 4 bytes data and 
AST_FMC_DMA_LENGTH do not need to be divided by 4.


ok. For that, I think you could replace aspeed_smc_dma_len() with

   return QEMU_ALIGN_UP(s->regs[R_DMA_LEN] + asc->dma_start_length, 4);

Thanks,

C.






If I change the model to match 1 bit / 4 bytes unit of the R_DMA_LEN register.
Linux fails to boot. I didn't dig further and this is something we need to
understand before committing.


I don't think this is necessary to add a Fixes tag because the problem
has been there for ages and no one reported it. Probably because the
only place DMA transfers are used is in U-Boot and transfers have a
non-zero length.


Currently, only supports dma length 4 bytes aligned.


Is this 4 bytes alignment new for the AST2700 or is this something you added
because the mask of DMA_LENGTH is now relaxed to match all addresses ?

#define DMA_LENGTH(val) ((val) & 0x01FF)

AST2700, AST2600 and AST1030 is from 1 byte to 1FF, so I change this Micro 
to fix data lost.
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2023.10/arch/arm/mach-aspeed/ast2700/spi.c#L186
Please see this line, it decrease dma_len 1 byte first then, set to DMA_LEN 
register because DMA_LEN is 0 which means should move 1 byte data if DMA 
enables for AST2600, AST1030 and AST2700.


Thanks,

C.


Only AST2500 need 4 bytes alignment for DMA Length. However, according to the 
design of sapped_smc_dma_rw function,
it utilizes address_space_stl_le API and it only works data 4 bytes alignment. 
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L889
For example,
If users want to move 0x101 data_length, after 0x100 data has been moved and 
remains 1 byte data need to be moved.
Please see this line program, 
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L940
```

RE: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

2024-04-30 Thread Jamin Lin
Hi Cedric,
> On 4/19/24 15:41, Cédric Le Goater wrote:
> > On 4/16/24 11:18, Jamin Lin wrote:
> >> DMA length is from 1 byte to 32MB for AST2600 and AST10x0 and DMA
> >> length is from 4 bytes to 32MB for AST2500.
> >>
> >> In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte
> >> data for AST2600 and AST10x0 and 4 bytes data for AST2500.
> >>> To support all ASPEED SOCs, adds dma_start_length parameter to store
> >> the start length, add helper routines function to compute the dma
> >> length and update DMA_LENGTH mask to "1FF" to fix dma moving
> >> incorrect data length issue.
> >
> > OK. There are two problems to address, the "zero" length transfer and
> > the DMA length unit, which is missing today. Newer SoC use a 1 bit /
> > byte and older ones, AST2400 and AST2500, use 1 bit / 4 bytes.
> >
> > We can introduce a AspeedSMCClass::dma_len_unit and rework the loop to :
> >
> >      do {
> >
> >    
> >
> >     if (s->regs[R_DMA_LEN]) {
> >      s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit;
> >      }
> >      } while (s->regs[R_DMA_LEN]);
> >
> > It should fix the current implementation.
> 
> 
> I checked what FW is doing on a QEMU ast2500-evb machine :
> 
>  U-Boot 2019.04-v00.04.12 (Sep 29 2022 - 10:40:37 +)
>  ...
> 
> Loading Kernel Image ... aspeed_smc_write @0x88 size 4:
> 0x80001000
>  aspeed_smc_write @0x84 size 4: 0x20100130
>  aspeed_smc_write @0x8c size 4: 0x3c6770
>  aspeed_smc_write @0x80 size 4: 0x1
>  aspeed_smc_dma_rw read flash:@0x00100130 dram:@0x1000
> size:0x003c6774
>  aspeed_smc_read @0x8 size 4: 0x800
>  aspeed_smc_write @0x80 size 4: 0x0
>  OK
> Loading Ramdisk to 8fef2000, end 8604 ... aspeed_smc_write
> @0x88 size 4: 0x8fef2000
>  aspeed_smc_write @0x84 size 4: 0x204cdde0
>  aspeed_smc_write @0x8c size 4: 0x10d604
>  aspeed_smc_write @0x80 size 4: 0x1
>  aspeed_smc_dma_rw read flash:@0x004cdde0 dram:@0xfef2000
> size:0x0010d608
>  aspeed_smc_read @0x8 size 4: 0x800
>  aspeed_smc_write @0x80 size 4: 0x0
>  OK
> Loading Device Tree to 8fee7000, end 8fef135e ... aspeed_smc_write
> @0x88 size 4: 0x8fee7000
>  aspeed_smc_write @0x84 size 4: 0x204c69b4
>  aspeed_smc_write @0x8c size 4: 0x7360
>  aspeed_smc_write @0x80 size 4: 0x1
>  aspeed_smc_dma_rw read flash:@0x004c69b4 dram:@0xfee7000
> size:0x7364
>  aspeed_smc_read @0x8 size 4: 0x800
>  aspeed_smc_write @0x80 size 4: 0x0
>  OK
> 
>  Starting kernel ...
> 
> It seems that the R_DMA_LEN register is set by FW without taking into account
> the length unit ( bit / 4 bytes). Would you know why ?
> 
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/lib/string.c#L559
This line make user input data length 4 bytes alignment.
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/arch/arm/mach-aspeed/ast2500/utils.S#L35
This line set the value of count parameter to AST_FMC_DNA_LENGTH.
AST_FMC_DMA_LENGTH is 4 bytes alignment value.
Example: input 4
((4+3)/4) * 4 --> (7/4) *4 ---> 4
If AST_FMC_DMA_LENGTH is 0, it means it should move 4 bytes data and 
AST_FMC_DMA_LENGTH do not need to be divided by 4. 

> If I change the model to match 1 bit / 4 bytes unit of the R_DMA_LEN register.
> Linux fails to boot. I didn't dig further and this is something we need to
> understand before committing.
> 
> > I don't think this is necessary to add a Fixes tag because the problem
> > has been there for ages and no one reported it. Probably because the
> > only place DMA transfers are used is in U-Boot and transfers have a
> > non-zero length.
> >
> >> Currently, only supports dma length 4 bytes aligned.
> 
> Is this 4 bytes alignment new for the AST2700 or is this something you added
> because the mask of DMA_LENGTH is now relaxed to match all addresses ?
> 
> #define DMA_LENGTH(val) ((val) & 0x01FF)
AST2700, AST2600 and AST1030 is from 1 byte to 1FF, so I change this Micro 
to fix data lost.
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2023.10/arch/arm/mach-aspeed/ast2700/spi.c#L186
Please see this line, it decrease dma_len 1 byte first then, set to DMA_LEN 
register because DMA_LEN is 0 which means should move 1 byte data if DMA 
enables for AST2600, AST1030 and AST2700. 
> 
> Thanks,
> 
> C.

Only AST2500 need 4 bytes alignment for DMA Length. However, according to the 
design of sapped_smc_dma_rw function,
it utilizes address_space_stl_le API and it only works data 4 bytes alignment. 
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L889 
For example,
If users want to move 0x101 data_length, after 0x100 data has been moved and 
remains 1 byte data need to be moved.
Please see this line program, 
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L940
```
s->regs[R_DMA_LEN] -= 4;
```
The value of s->regs[R_DMA_LEN] becomes 0xXX because it is uint32_t 
data type and this while loop run 

Re: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

2024-04-30 Thread Cédric Le Goater

On 4/19/24 15:41, Cédric Le Goater wrote:

On 4/16/24 11:18, Jamin Lin wrote:

DMA length is from 1 byte to 32MB for AST2600 and AST10x0
and DMA length is from 4 bytes to 32MB for AST2500.

In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte
data for AST2600 and AST10x0 and 4 bytes data for AST2500.

To support all ASPEED SOCs, adds dma_start_length parameter to store

the start length, add helper routines function to compute the dma length
and update DMA_LENGTH mask to "1FF" to fix dma moving
incorrect data length issue.


OK. There are two problems to address, the "zero" length transfer and
the DMA length unit, which is missing today. Newer SoC use a 1 bit / byte
and older ones, AST2400 and AST2500, use 1 bit / 4 bytes.

We can introduce a AspeedSMCClass::dma_len_unit and rework the loop to :

     do {

   

    if (s->regs[R_DMA_LEN]) {
     s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit;
     }
     } while (s->regs[R_DMA_LEN]);

It should fix the current implementation.



I checked what FW is doing on a QEMU ast2500-evb machine :
 
U-Boot 2019.04-v00.04.12 (Sep 29 2022 - 10:40:37 +)

...

   Loading Kernel Image ... aspeed_smc_write @0x88 size 4: 0x80001000

aspeed_smc_write @0x84 size 4: 0x20100130
aspeed_smc_write @0x8c size 4: 0x3c6770
aspeed_smc_write @0x80 size 4: 0x1
aspeed_smc_dma_rw read flash:@0x00100130 dram:@0x1000 size:0x003c6774
aspeed_smc_read @0x8 size 4: 0x800
aspeed_smc_write @0x80 size 4: 0x0
OK
   Loading Ramdisk to 8fef2000, end 8604 ... aspeed_smc_write @0x88 
size 4: 0x8fef2000
aspeed_smc_write @0x84 size 4: 0x204cdde0
aspeed_smc_write @0x8c size 4: 0x10d604
aspeed_smc_write @0x80 size 4: 0x1
aspeed_smc_dma_rw read flash:@0x004cdde0 dram:@0xfef2000 size:0x0010d608
aspeed_smc_read @0x8 size 4: 0x800
aspeed_smc_write @0x80 size 4: 0x0
OK
   Loading Device Tree to 8fee7000, end 8fef135e ... aspeed_smc_write @0x88 
size 4: 0x8fee7000
aspeed_smc_write @0x84 size 4: 0x204c69b4
aspeed_smc_write @0x8c size 4: 0x7360
aspeed_smc_write @0x80 size 4: 0x1
aspeed_smc_dma_rw read flash:@0x004c69b4 dram:@0xfee7000 size:0x7364
aspeed_smc_read @0x8 size 4: 0x800
aspeed_smc_write @0x80 size 4: 0x0
OK

Starting kernel ...

It seems that the R_DMA_LEN register is set by FW without taking

into account the length unit ( bit / 4 bytes). Would you know why ?

If I change the model to match 1 bit / 4 bytes unit of the R_DMA_LEN
register. Linux fails to boot. I didn't dig further and this is
something we need to understand before committing.


I don't think this is necessary to add a Fixes tag because the problem
has been there for ages and no one reported it. Probably because the
only place DMA transfers are used is in U-Boot and transfers have a
non-zero length.


Currently, only supports dma length 4 bytes aligned.


Is this 4 bytes alignment new for the AST2700 or is this something
you added because the mask of DMA_LENGTH is now relaxed to match
all addresses ?

#define DMA_LENGTH(val) ((val) & 0x01FF)

Thanks,

C.



Re: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

2024-04-19 Thread Cédric Le Goater

On 4/16/24 11:18, Jamin Lin wrote:

DMA length is from 1 byte to 32MB for AST2600 and AST10x0
and DMA length is from 4 bytes to 32MB for AST2500.

In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte
data for AST2600 and AST10x0 and 4 bytes data for AST2500.

To support all ASPEED SOCs, adds dma_start_length parameter to store

the start length, add helper routines function to compute the dma length
and update DMA_LENGTH mask to "1FF" to fix dma moving
incorrect data length issue.


OK. There are two problems to address, the "zero" length transfer and
the DMA length unit, which is missing today. Newer SoC use a 1 bit / byte
and older ones, AST2400 and AST2500, use 1 bit / 4 bytes.

We can introduce a AspeedSMCClass::dma_len_unit and rework the loop to :

do {

  

   if (s->regs[R_DMA_LEN]) {
s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit;
}
} while (s->regs[R_DMA_LEN]);

It should fix the current implementation.

I don't think this is necessary to add a Fixes tag because the problem
has been there for ages and no one reported it. Probably because the
only place DMA transfers are used is in U-Boot and transfers have a
non-zero length.


Currently, only supports dma length 4 bytes aligned.


this looks like a third topic. So the minimum value R_DMA_LEN should
have on the AST2600 SoC and above is '3'. I would opt to replace the
DMA_LENGTH macro with a dma_length_sanitize() helper to fix the software
input of R_DMA_LEN.


Thanks,

C.


 

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 
---
  hw/ssi/aspeed_smc.c | 52 -
  include/hw/ssi/aspeed_smc.h |  1 +
  2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 8a8d77b480..71abc7a2d8 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -178,13 +178,17 @@
   * DMA flash addresses should be 4 bytes aligned and the valid address
   * range is 0x2000 - 0x2FFF.
   *
- * DMA length is from 4 bytes to 32MB
+ * DMA length is from 4 bytes to 32MB (AST2500)
   *   0: 4 bytes
   *   0x7F: 32M bytes
+ *
+ * DMA length is from 1 byte to 32MB (AST2600, AST10x0)
+ *   0: 1 byte
+ *   0x1FF: 32M bytes
   */
  #define DMA_DRAM_ADDR(asc, val)   ((val) & (asc)->dma_dram_mask)
  #define DMA_FLASH_ADDR(asc, val)  ((val) & (asc)->dma_flash_mask)
-#define DMA_LENGTH(val) ((val) & 0x01FC)
+#define DMA_LENGTH(val) ((val) & 0x01FF)
  
  /* Flash opcodes. */

  #define SPI_OP_READ   0x03/* Read data bytes (low frequency) */
@@ -843,6 +847,24 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState 
*s)
  }
  }
  
+static uint32_t aspeed_smc_dma_len(AspeedSMCState *s)

+{
+AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
+uint32_t dma_len;
+uint32_t extra;
+
+dma_len = s->regs[R_DMA_LEN] + asc->dma_start_length;
+
+/* dma length 4 bytes aligned */
+extra = dma_len % 4;
+
+if (extra != 0) {
+dma_len += 4 - extra;
+}
+
+return dma_len;
+}
+
  /*
   * Accumulate the result of the reads to provide a checksum that will
   * be used to validate the read timing settings.
@@ -850,6 +872,7 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState 
*s)
  static void aspeed_smc_dma_checksum(AspeedSMCState *s)
  {
  MemTxResult result;
+uint32_t dma_len;
  uint32_t data;
  
  if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {

@@ -861,7 +884,9 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
  aspeed_smc_dma_calibration(s);
  }
  
-while (s->regs[R_DMA_LEN]) {

+dma_len = aspeed_smc_dma_len(s);
+
+while (dma_len) {
  data = address_space_ldl_le(>flash_as, s->regs[R_DMA_FLASH_ADDR],
  MEMTXATTRS_UNSPECIFIED, );
  if (result != MEMTX_OK) {
@@ -877,7 +902,8 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
   */
  s->regs[R_DMA_CHECKSUM] += data;
  s->regs[R_DMA_FLASH_ADDR] += 4;
-s->regs[R_DMA_LEN] -= 4;
+dma_len -= 4;
+s->regs[R_DMA_LEN] = dma_len;
  }
  
  if (s->inject_failure && aspeed_smc_inject_read_failure(s)) {

@@ -889,14 +915,17 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
  static void aspeed_smc_dma_rw(AspeedSMCState *s)
  {
  MemTxResult result;
+uint32_t dma_len;
  uint32_t data;
  
+dma_len = aspeed_smc_dma_len(s);

+
  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]) {
+dma_len);
+while (dma_len) {
  if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
  data = address_space_ldl_le(>dram_as, s->regs[R_DMA_DRAM_ADDR],