> Subject: RE: [PATCH v1 08/22] hw/misc/aspeed_hace: Support DMA 64 bits > dram address. > > Hi Cédric > > > > > > > > >> The SG_LIST_ADDR_MASK needs an update though. AFAICT, it's bigger > > > >> on AST2700. > > > > > > > > The value of SG_LIST_ADDR_MASK was wrong for AST2700, AST2600 and > > > AST1030. > > > > The correct value should be 0x7FFFFFF8. > > > > Will create a new patch to fix it. > > > > Please see patch 4 comments. > > > > By the way, AST2500 do not support SG mode. > > > > > > Should we introduce a class attribute then ? > > > > > Can I modify SG_LIST_ADDR_MASK directly? > > > > In this model, hash_mask is set to 0x000003ff for AST2500, which > > disables support for SG mode and SHA512: > > As a result, the model does not handle SG mode for AST2500. > > > > https://github.com/qemu/qemu/blob/master/hw/misc/aspeed_hace.c#L529 > > ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */ > > > > I would like to correct my previous investigation. > > According to the datasheet, the source buffer address list must be 8-byte > aligned. Therefore, the definition of HACE20 should be as follows: > Bit 31: Reserved 0 > Bit 30:3 base address of sg list > Bit 2:0 Reserved 0 > > In the current implementation of SG mode, the "local src" variable represents > the buffer address, and each scatter-gather entry has a size of 8 bytes
Sorry typo --> the "local src" variable represents the buffer list address buffer list address1 addr --> 8-byte aligned". len buf_addr A --> do not need to be 8-byte aligned. buffer list address2 addr len buf_addr B --> do not need to be 8-byte aligned. Addr A: Data Addr B: Data Thanks-Jamin > (SG_LIST_ENTRY_SIZE). > As a result, we only need to ensure that the "local src variable itself is > 8-byte > aligned". > The actual buffer addresses referenced by the SG list entries do not need to > be > 8-byte aligned. > > ``` > src = s->regs[R_HASH_SRC] + (i * SG_LIST_ENTRY_SIZE); ``` > > We only need to ensure that the starting src address is aligned to 8 bytes. > > ``` > len = address_space_ldl_le(&s->dram_as, src, > MEMTXATTRS_UNSPECIFIED, > NULL); > > addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE, > MEMTXATTRS_UNSPECIFIED, > NULL); > addr &= SG_LIST_ADDR_MASK; > ``` > > The local addr variable represents the buffer address and does not need to be > 8-byte aligned. > I tried removing this line(addr &= SG_LIST_ADDR_MASK), but encountered a > failure in the AST2600 test. > This is because the AST2600 DRAM base address is 0x80000000, and the > original code applies "addr & 0x7FFFFFFF" to convert the buffer address to > 0x0, > which corresponds to the "DRAM offset 0". In this case, there is no need to > perform (addr - 0x80000000) to compute the DRAM offset on AST2600. > > As a result, I will not modify SG_LIST_ADDR_MASK in this patch. > If you still wish to improve this part, I can create a separate patch series > to > address it, since the current patch set has already become quite large (29 > patches will be re-sent in v3). > > Thanks-Jamin > > > Thanks-Jamin > > > Thanks, > > > > > > C.
