Re: [PATCH 1/7] hw/dma: add K230 gsdma
Tao Ding <[email protected]> Sun, 26 Jul 2026 21:40:48 +0800
| Newsgroups | org.nongnu.qemu-riscv,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format. --------------WPfMADm9LVBV7VF0omcZVp7w Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/26/2026 1:24 PM, Junze Cao wrote: > On Wed, 22 Jul 2026 00:13:29 +0800, Tao Ding<[email protected]> wrote: >> diff --git a/hw/dma/k230_gsdma.c b/hw/dma/k230_gsdma.c >> new file mode 100644 >> index 0000000000..1f76f62fa0 >> --- /dev/null >> +++ b/hw/dma/k230_gsdma.c >> @@ -0,0 +1,532 @@ >> [ ... skip 75 lines ... ] >> + /* Stride line size required for 2d mode */ >> + uint32_t line_space = extract32(line_cfg, 16, 16); >> + >> + bool dat_mode = c->cfg & K230_GSDMA_CH_CFG_DAT_MODE; >> + bool src_fixed = c->cfg & K230_GSDMA_CH_CFG_SRC_FIXED; >> + bool dst_fixed = c->cfg & K230_GSDMA_CH_CFG_DST_FIXED; > The handling for chx_dat_endian is completely missing here. Could you please check this register field in the TRM and implement its functionality? It will be added in the next patch. >> [ ... skip 39 lines ... ] >> + remaining -= chunk; >> + } >> + >> + if (!dat_mode) { >> + if (llt_cfg & K230_GSDMA_LLT_2D_MODE) { /*2d mode*/ >> + src += line_size + line_space; > The 2D mode semantics here don't match the TRM. > According to the TRM, when in 2D mode and SRC_FIXED equals 1, the correct behaviour is that SRC_ADDR should not increment. > There is currently no check here to determine whether SRC_FIXED is set to 1. yes, it's a bug. >> [ ... skip 62 lines ... ] >> + return; >> + } >> + >> + addr = next; >> + } >> + > When CHx_CTL.START is written via MMIO, the entire llt is processed synchronously. During this period, the vCPU is unable to stop, resume, or reset command. This could potentially cause QEMU to be blocked here for an indefinite amount of time. > > For the implementation, please refer to the reference in i8257.c. Yes, the transmission is processed synchronously after startup. Only when the software is set incorrectly, such as LLT being a circular linked list. DMA may fall into a dead loop, in which case stop and other operations cannot be performed. I think we can add a limit on the maximum number of LLT processing times. For example, when transmitting 1GB, assuming that LLT is performed every 4KB, the number of LLTs is 1GB/4KB=0x40000. >> [ ... skip 315 lines ... ] >> +static void k230_gsdma_class_init(ObjectClass *klass, const void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->realize = k230_gsdma_realize; >> + device_class_set_legacy_reset(dc, k230_gsdma_reset); > This interface is no longer recommended for use. For further details, please refer to the comments for this interface in qdev.h. > --------------WPfMADm9LVBV7VF0omcZVp7w Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit <!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <p><br> </p> <pre class="moz-cite-prefix">On 7/26/2026 1:24 PM, Junze Cao wrote: </pre> <blockquote type="cite" cite="mid:178504345275.903105.7911994626854937851.b4-review@b4"> <pre wrap="" class="moz-quote-pre">On Wed, 22 Jul 2026 00:13:29 +0800, Tao Ding <a class="moz-txt-link-rfc2396E" href="mailto:[email protected]"><[email protected]></a> wrote: </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">diff --git a/hw/dma/k230_gsdma.c b/hw/dma/k230_gsdma.c new file mode 100644 index 0000000000..1f76f62fa0 --- /dev/null +++ b/hw/dma/k230_gsdma.c @@ -0,0 +1,532 @@ [ ... skip 75 lines ... ] + /* Stride line size required for 2d mode */ + uint32_t line_space = extract32(line_cfg, 16, 16); + + bool dat_mode = c->cfg & K230_GSDMA_CH_CFG_DAT_MODE; + bool src_fixed = c->cfg & K230_GSDMA_CH_CFG_SRC_FIXED; + bool dst_fixed = c->cfg & K230_GSDMA_CH_CFG_DST_FIXED; </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> The handling for chx_dat_endian is completely missing here. Could you please check this register field in the TRM and implement its functionality?</pre> </blockquote> <pre>It will be added in the next patch.</pre> <blockquote type="cite" cite="mid:178504345275.903105.7911994626854937851.b4-review@b4"> <pre wrap="" class="moz-quote-pre"> </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">[ ... skip 39 lines ... ] + remaining -= chunk; + } + + if (!dat_mode) { + if (llt_cfg & K230_GSDMA_LLT_2D_MODE) { /*2d mode*/ + src += line_size + line_space; </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> The 2D mode semantics here don't match the TRM. According to the TRM, when in 2D mode and SRC_FIXED equals 1, the correct behaviour is that SRC_ADDR should not increment. There is currently no check here to determine whether SRC_FIXED is set to 1. </pre> </blockquote> <pre>yes, it's a bug.</pre> <blockquote type="cite" cite="mid:178504345275.903105.7911994626854937851.b4-review@b4"> <pre wrap="" class="moz-quote-pre"> </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">[ ... skip 62 lines ... ] + return; + } + + addr = next; + } + </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> When CHx_CTL.START is written via MMIO, the entire llt is processed synchronously. During this period, the vCPU is unable to stop, resume, or reset command. This could potentially cause QEMU to be blocked here for an indefinite amount of time. For the implementation, please refer to the reference in i8257.c. </pre> </blockquote> <pre>Yes, the transmission is processed synchronously after startup. Only when the software is set incorrectly, such as LLT being a circular linked list. DMA may fall into a dead loop, in which case stop and other operations cannot be performed. I think we can add a limit on the maximum number of LLT processing times. For example, when transmitting 1GB, assuming that LLT is performed every 4KB, the number of LLTs is 1GB/4KB=0x40000. </pre> <blockquote type="cite" cite="mid:178504345275.903105.7911994626854937851.b4-review@b4"> <pre wrap="" class="moz-quote-pre"> </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">[ ... skip 315 lines ... ] +static void k230_gsdma_class_init(ObjectClass *klass, const void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = k230_gsdma_realize; + device_class_set_legacy_reset(dc, k230_gsdma_reset); </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> This interface is no longer recommended for use. For further details, please refer to the comments for this interface in qdev.h. </pre> </blockquote> </body> </html> --------------WPfMADm9LVBV7VF0omcZVp7w--