Re: Handle openbsd,dma-constraint on armv7
> Date: Tue, 5 Oct 2021 12:42:21 +
> From: Visa Hankala
>
> On Mon, Oct 04, 2021 at 05:04:11PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 4 Oct 2021 13:42:48 +
> > > From: Visa Hankala
> > >
> > > On the Zynq-7000, the lowest 512KiB of physical address space usually
> > > contains RAM that is usable by the CPUs. However, many other bus
> > > masters, such as the Ethernet and SDIO controllers, are not able to
> > > access the 256KiB range that starts at physical address 0x4.
> > >
> > > So far I have used a device tree that says that RAM starts at 0x8,
> > > to avoid the DMA hole. This is unconventional, though. Typically the
> > > memory node for Zynq-7000 specifies 0x0 as the starting address for RAM.
> > >
> > > I think armv7 DMA constraint should be adjusted on the Zynq-7000 so that
> > > less device tree customization would be needed.
> > >
> > > This diff makes armv7 efiboot and kernel handle the
> > > openbsd,dma-constraint device tree property, with a tweak for the Zynq.
> > > The code is similar to what is already present on arm64 and riscv64.
> > >
> > > OK?
> >
> > Hmm. How does Linux know it can't do DMA to that memory range?
> > Normally that is done through a dma-ranges property in the device
> > tree, and my idea has always been to add code to parse these
> > properties to determine the valid DMA constraints. The reason there
> > is special code for the rpi4 is because the initial device trees for
> > the rpi4 didn't have those dma-ranges properties.
>
> Linux reserves the first 512KiB in Zynq platform init code to prevent
> the region from being used.
So basically they do what we try to do here, but in the kernel. With
that explanation, this diff (with the suggested change) is ok kettenis@
Cheers,
Mark
> > I'm not necessarily against this diff going in, just curious if there
> > is a better way.
> >
> > > Index: arch/armv7/armv7/armv7_machdep.c
> > > ===
> > > RCS file: src/sys/arch/armv7/armv7/armv7_machdep.c,v
> > > retrieving revision 1.63
> > > diff -u -p -r1.63 armv7_machdep.c
> > > --- arch/armv7/armv7/armv7_machdep.c 25 Mar 2021 04:12:01 -
> > > 1.63
> > > +++ arch/armv7/armv7/armv7_machdep.c 4 Oct 2021 13:32:11 -
> > > @@ -453,6 +453,12 @@ initarm(void *arg0, void *arg1, void *ar
> > > len = fdt_node_property(node, "openbsd,uefi-mmap-desc-ver",
> > > );
> > > if (len == sizeof(mmap_desc_ver))
> > > mmap_desc_ver = bemtoh32((uint32_t *)prop);
> > > +
> > > + len = fdt_node_property(node, "openbsd,dma-constraint", );
> > > + if (len == sizeof(uint64_t[2])) {
> > > + dma_constraint.ucr_low = bemtoh64((uint64_t *)prop);
> > > + dma_constraint.ucr_high = bemtoh64((uint64_t *)prop +
> > > 1);
> > > + }
> > > }
> > >
> > > process_kernel_args();
> > > Index: arch/armv7/stand/efiboot/conf.c
> > > ===
> > > RCS file: src/sys/arch/armv7/stand/efiboot/conf.c,v
> > > retrieving revision 1.31
> > > diff -u -p -r1.31 conf.c
> > > --- arch/armv7/stand/efiboot/conf.c 10 Jun 2021 22:17:58 -
> > > 1.31
> > > +++ arch/armv7/stand/efiboot/conf.c 4 Oct 2021 13:32:11 -
> > > @@ -42,7 +42,7 @@
> > > #include "efidev.h"
> > > #include "efipxe.h"
> > >
> > > -const char version[] = "1.18";
> > > +const char version[] = "1.19";
> > > int debug = 0;
> > >
> > > struct fs_ops file_system[] = {
> > > Index: arch/armv7/stand/efiboot/efiboot.c
> > > ===
> > > RCS file: src/sys/arch/armv7/stand/efiboot/efiboot.c,v
> > > retrieving revision 1.34
> > > diff -u -p -r1.34 efiboot.c
> > > --- arch/armv7/stand/efiboot/efiboot.c7 Jun 2021 21:18:31 -
> > > 1.34
> > > +++ arch/armv7/stand/efiboot/efiboot.c4 Oct 2021 13:32:11 -
> > > @@ -435,6 +435,28 @@ efi_framebuffer(void)
> > > sizeof(framebuffer_path));
> > > }
> > >
> > > +uint64_t dma_constraint[2] = { 0, -1 };
> > > +
> > > +void
> > > +efi_dma_constraint(void)
> > > +{
> > > + void *node;
> > > +
> > > + /* Raspberry Pi 4 is "special". */
> > > + node = fdt_find_node("/");
> > > + if (fdt_node_is_compatible(node, "brcm,bcm2711"))
> > > + dma_constraint[1] = htobe64(0x3bff);
> > > +
> > > + /* Not all bus masters can access 0x4-0x7 on Zynq-7000. */
>
> Even range 0x0-0x3 has fine print on it, so I will change the above
> comment to:
>
> /* Not all bus masters can access 0x0-0x7 on Zynq-7000. */
>
> > > + if (fdt_node_is_compatible(node, "xlnx,zynq-7000"))
> > > + dma_constraint[0] = htobe64(0x0008);
> > > +
> > > + /* Pass DMA constraint. */
> > > + node = fdt_find_node("/chosen");
> > > + fdt_node_add_property(node, "openbsd,dma-constraint",
> > > + dma_constraint, sizeof(dma_constraint));
> > >
Re: Handle openbsd,dma-constraint on armv7
On Mon, Oct 04, 2021 at 05:04:11PM +0200, Mark Kettenis wrote:
> > Date: Mon, 4 Oct 2021 13:42:48 +
> > From: Visa Hankala
> >
> > On the Zynq-7000, the lowest 512KiB of physical address space usually
> > contains RAM that is usable by the CPUs. However, many other bus
> > masters, such as the Ethernet and SDIO controllers, are not able to
> > access the 256KiB range that starts at physical address 0x4.
> >
> > So far I have used a device tree that says that RAM starts at 0x8,
> > to avoid the DMA hole. This is unconventional, though. Typically the
> > memory node for Zynq-7000 specifies 0x0 as the starting address for RAM.
> >
> > I think armv7 DMA constraint should be adjusted on the Zynq-7000 so that
> > less device tree customization would be needed.
> >
> > This diff makes armv7 efiboot and kernel handle the
> > openbsd,dma-constraint device tree property, with a tweak for the Zynq.
> > The code is similar to what is already present on arm64 and riscv64.
> >
> > OK?
>
> Hmm. How does Linux know it can't do DMA to that memory range?
> Normally that is done through a dma-ranges property in the device
> tree, and my idea has always been to add code to parse these
> properties to determine the valid DMA constraints. The reason there
> is special code for the rpi4 is because the initial device trees for
> the rpi4 didn't have those dma-ranges properties.
Linux reserves the first 512KiB in Zynq platform init code to prevent
the region from being used.
> I'm not necessarily against this diff going in, just curious if there
> is a better way.
>
> > Index: arch/armv7/armv7/armv7_machdep.c
> > ===
> > RCS file: src/sys/arch/armv7/armv7/armv7_machdep.c,v
> > retrieving revision 1.63
> > diff -u -p -r1.63 armv7_machdep.c
> > --- arch/armv7/armv7/armv7_machdep.c25 Mar 2021 04:12:01 -
> > 1.63
> > +++ arch/armv7/armv7/armv7_machdep.c4 Oct 2021 13:32:11 -
> > @@ -453,6 +453,12 @@ initarm(void *arg0, void *arg1, void *ar
> > len = fdt_node_property(node, "openbsd,uefi-mmap-desc-ver",
> > );
> > if (len == sizeof(mmap_desc_ver))
> > mmap_desc_ver = bemtoh32((uint32_t *)prop);
> > +
> > + len = fdt_node_property(node, "openbsd,dma-constraint", );
> > + if (len == sizeof(uint64_t[2])) {
> > + dma_constraint.ucr_low = bemtoh64((uint64_t *)prop);
> > + dma_constraint.ucr_high = bemtoh64((uint64_t *)prop +
> > 1);
> > + }
> > }
> >
> > process_kernel_args();
> > Index: arch/armv7/stand/efiboot/conf.c
> > ===
> > RCS file: src/sys/arch/armv7/stand/efiboot/conf.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 conf.c
> > --- arch/armv7/stand/efiboot/conf.c 10 Jun 2021 22:17:58 - 1.31
> > +++ arch/armv7/stand/efiboot/conf.c 4 Oct 2021 13:32:11 -
> > @@ -42,7 +42,7 @@
> > #include "efidev.h"
> > #include "efipxe.h"
> >
> > -const char version[] = "1.18";
> > +const char version[] = "1.19";
> > intdebug = 0;
> >
> > struct fs_ops file_system[] = {
> > Index: arch/armv7/stand/efiboot/efiboot.c
> > ===
> > RCS file: src/sys/arch/armv7/stand/efiboot/efiboot.c,v
> > retrieving revision 1.34
> > diff -u -p -r1.34 efiboot.c
> > --- arch/armv7/stand/efiboot/efiboot.c 7 Jun 2021 21:18:31 -
> > 1.34
> > +++ arch/armv7/stand/efiboot/efiboot.c 4 Oct 2021 13:32:11 -
> > @@ -435,6 +435,28 @@ efi_framebuffer(void)
> > sizeof(framebuffer_path));
> > }
> >
> > +uint64_t dma_constraint[2] = { 0, -1 };
> > +
> > +void
> > +efi_dma_constraint(void)
> > +{
> > + void *node;
> > +
> > + /* Raspberry Pi 4 is "special". */
> > + node = fdt_find_node("/");
> > + if (fdt_node_is_compatible(node, "brcm,bcm2711"))
> > + dma_constraint[1] = htobe64(0x3bff);
> > +
> > + /* Not all bus masters can access 0x4-0x7 on Zynq-7000. */
Even range 0x0-0x3 has fine print on it, so I will change the above
comment to:
/* Not all bus masters can access 0x0-0x7 on Zynq-7000. */
> > + if (fdt_node_is_compatible(node, "xlnx,zynq-7000"))
> > + dma_constraint[0] = htobe64(0x0008);
> > +
> > + /* Pass DMA constraint. */
> > + node = fdt_find_node("/chosen");
> > + fdt_node_add_property(node, "openbsd,dma-constraint",
> > + dma_constraint, sizeof(dma_constraint));
> > +}
> > +
> > void
> > efi_console(void)
> > {
> > @@ -515,6 +537,7 @@ efi_makebootargs(char *bootargs, int how
> >
> > efi_framebuffer();
> > efi_console();
> > + efi_dma_constraint();
> >
> > fdt_finalize();
> >
> >
> >
>
Re: Handle openbsd,dma-constraint on armv7
> Date: Mon, 4 Oct 2021 13:42:48 +
> From: Visa Hankala
>
> On the Zynq-7000, the lowest 512KiB of physical address space usually
> contains RAM that is usable by the CPUs. However, many other bus
> masters, such as the Ethernet and SDIO controllers, are not able to
> access the 256KiB range that starts at physical address 0x4.
>
> So far I have used a device tree that says that RAM starts at 0x8,
> to avoid the DMA hole. This is unconventional, though. Typically the
> memory node for Zynq-7000 specifies 0x0 as the starting address for RAM.
>
> I think armv7 DMA constraint should be adjusted on the Zynq-7000 so that
> less device tree customization would be needed.
>
> This diff makes armv7 efiboot and kernel handle the
> openbsd,dma-constraint device tree property, with a tweak for the Zynq.
> The code is similar to what is already present on arm64 and riscv64.
>
> OK?
Hmm. How does Linux know it can't do DMA to that memory range?
Normally that is done through a dma-ranges property in the device
tree, and my idea has always been to add code to parse these
properties to determine the valid DMA constraints. The reason there
is special code for the rpi4 is because the initial device trees for
the rpi4 didn't have those dma-ranges properties.
I'm not necessarily against this diff going in, just curious if there
is a better way.
> Index: arch/armv7/armv7/armv7_machdep.c
> ===
> RCS file: src/sys/arch/armv7/armv7/armv7_machdep.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 armv7_machdep.c
> --- arch/armv7/armv7/armv7_machdep.c 25 Mar 2021 04:12:01 - 1.63
> +++ arch/armv7/armv7/armv7_machdep.c 4 Oct 2021 13:32:11 -
> @@ -453,6 +453,12 @@ initarm(void *arg0, void *arg1, void *ar
> len = fdt_node_property(node, "openbsd,uefi-mmap-desc-ver",
> );
> if (len == sizeof(mmap_desc_ver))
> mmap_desc_ver = bemtoh32((uint32_t *)prop);
> +
> + len = fdt_node_property(node, "openbsd,dma-constraint", );
> + if (len == sizeof(uint64_t[2])) {
> + dma_constraint.ucr_low = bemtoh64((uint64_t *)prop);
> + dma_constraint.ucr_high = bemtoh64((uint64_t *)prop +
> 1);
> + }
> }
>
> process_kernel_args();
> Index: arch/armv7/stand/efiboot/conf.c
> ===
> RCS file: src/sys/arch/armv7/stand/efiboot/conf.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 conf.c
> --- arch/armv7/stand/efiboot/conf.c 10 Jun 2021 22:17:58 - 1.31
> +++ arch/armv7/stand/efiboot/conf.c 4 Oct 2021 13:32:11 -
> @@ -42,7 +42,7 @@
> #include "efidev.h"
> #include "efipxe.h"
>
> -const char version[] = "1.18";
> +const char version[] = "1.19";
> int debug = 0;
>
> struct fs_ops file_system[] = {
> Index: arch/armv7/stand/efiboot/efiboot.c
> ===
> RCS file: src/sys/arch/armv7/stand/efiboot/efiboot.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 efiboot.c
> --- arch/armv7/stand/efiboot/efiboot.c7 Jun 2021 21:18:31 -
> 1.34
> +++ arch/armv7/stand/efiboot/efiboot.c4 Oct 2021 13:32:11 -
> @@ -435,6 +435,28 @@ efi_framebuffer(void)
> sizeof(framebuffer_path));
> }
>
> +uint64_t dma_constraint[2] = { 0, -1 };
> +
> +void
> +efi_dma_constraint(void)
> +{
> + void *node;
> +
> + /* Raspberry Pi 4 is "special". */
> + node = fdt_find_node("/");
> + if (fdt_node_is_compatible(node, "brcm,bcm2711"))
> + dma_constraint[1] = htobe64(0x3bff);
> +
> + /* Not all bus masters can access 0x4-0x7 on Zynq-7000. */
> + if (fdt_node_is_compatible(node, "xlnx,zynq-7000"))
> + dma_constraint[0] = htobe64(0x0008);
> +
> + /* Pass DMA constraint. */
> + node = fdt_find_node("/chosen");
> + fdt_node_add_property(node, "openbsd,dma-constraint",
> + dma_constraint, sizeof(dma_constraint));
> +}
> +
> void
> efi_console(void)
> {
> @@ -515,6 +537,7 @@ efi_makebootargs(char *bootargs, int how
>
> efi_framebuffer();
> efi_console();
> + efi_dma_constraint();
>
> fdt_finalize();
>
>
>
Re: Handle openbsd,dma-constraint on armv7
Am Mon, Oct 04, 2021 at 01:42:48PM + schrieb Visa Hankala:
> On the Zynq-7000, the lowest 512KiB of physical address space usually
> contains RAM that is usable by the CPUs. However, many other bus
> masters, such as the Ethernet and SDIO controllers, are not able to
> access the 256KiB range that starts at physical address 0x4.
>
> So far I have used a device tree that says that RAM starts at 0x8,
> to avoid the DMA hole. This is unconventional, though. Typically the
> memory node for Zynq-7000 specifies 0x0 as the starting address for RAM.
>
> I think armv7 DMA constraint should be adjusted on the Zynq-7000 so that
> less device tree customization would be needed.
>
> This diff makes armv7 efiboot and kernel handle the
> openbsd,dma-constraint device tree property, with a tweak for the Zynq.
> The code is similar to what is already present on arm64 and riscv64.
>
> OK?
Heh, I just saw that the dma constraint range is paddr_t, so on ARMv7
it's 32-bit values. Hence it makes sense that you're not using sizeof(
dma_constraint).
Using the same size in the openbsd,dma-constrant API as arm64 does make
sense to me, so while it might looks less nice than on arm64, I feel it
is the sane way.
Might want to see if kettenis agrees, since he built it, but otherwise
ok patrick@.
> Index: arch/armv7/armv7/armv7_machdep.c
> ===
> RCS file: src/sys/arch/armv7/armv7/armv7_machdep.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 armv7_machdep.c
> --- arch/armv7/armv7/armv7_machdep.c 25 Mar 2021 04:12:01 - 1.63
> +++ arch/armv7/armv7/armv7_machdep.c 4 Oct 2021 13:32:11 -
> @@ -453,6 +453,12 @@ initarm(void *arg0, void *arg1, void *ar
> len = fdt_node_property(node, "openbsd,uefi-mmap-desc-ver",
> );
> if (len == sizeof(mmap_desc_ver))
> mmap_desc_ver = bemtoh32((uint32_t *)prop);
> +
> + len = fdt_node_property(node, "openbsd,dma-constraint", );
> + if (len == sizeof(uint64_t[2])) {
> + dma_constraint.ucr_low = bemtoh64((uint64_t *)prop);
> + dma_constraint.ucr_high = bemtoh64((uint64_t *)prop +
> 1);
> + }
> }
>
> process_kernel_args();
> Index: arch/armv7/stand/efiboot/conf.c
> ===
> RCS file: src/sys/arch/armv7/stand/efiboot/conf.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 conf.c
> --- arch/armv7/stand/efiboot/conf.c 10 Jun 2021 22:17:58 - 1.31
> +++ arch/armv7/stand/efiboot/conf.c 4 Oct 2021 13:32:11 -
> @@ -42,7 +42,7 @@
> #include "efidev.h"
> #include "efipxe.h"
>
> -const char version[] = "1.18";
> +const char version[] = "1.19";
> int debug = 0;
>
> struct fs_ops file_system[] = {
> Index: arch/armv7/stand/efiboot/efiboot.c
> ===
> RCS file: src/sys/arch/armv7/stand/efiboot/efiboot.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 efiboot.c
> --- arch/armv7/stand/efiboot/efiboot.c7 Jun 2021 21:18:31 -
> 1.34
> +++ arch/armv7/stand/efiboot/efiboot.c4 Oct 2021 13:32:11 -
> @@ -435,6 +435,28 @@ efi_framebuffer(void)
> sizeof(framebuffer_path));
> }
>
> +uint64_t dma_constraint[2] = { 0, -1 };
> +
> +void
> +efi_dma_constraint(void)
> +{
> + void *node;
> +
> + /* Raspberry Pi 4 is "special". */
> + node = fdt_find_node("/");
> + if (fdt_node_is_compatible(node, "brcm,bcm2711"))
> + dma_constraint[1] = htobe64(0x3bff);
> +
> + /* Not all bus masters can access 0x4-0x7 on Zynq-7000. */
> + if (fdt_node_is_compatible(node, "xlnx,zynq-7000"))
> + dma_constraint[0] = htobe64(0x0008);
> +
> + /* Pass DMA constraint. */
> + node = fdt_find_node("/chosen");
> + fdt_node_add_property(node, "openbsd,dma-constraint",
> + dma_constraint, sizeof(dma_constraint));
> +}
> +
> void
> efi_console(void)
> {
> @@ -515,6 +537,7 @@ efi_makebootargs(char *bootargs, int how
>
> efi_framebuffer();
> efi_console();
> + efi_dma_constraint();
>
> fdt_finalize();
>
>
