Message ID | 20220705192320.415861-11-oleksandr.suvorov@foundries.io |
---|---|
State | Superseded |
Delegated to: | Michal Simek |
Headers | show |
Series | fpga: zynqmp: Adding support of loading authenticated images | expand |
On 7/5/22 21:23, Oleksandr Suvorov wrote: > Optimize function code preparing to add secure bitstream types > support. Can you please extend this? I understand what you do below but better description will be good. > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io> > Tested-by: Ricardo Salveti <ricardo@foundries.io> > Tested-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com> > --- > > (no changes since v1) > > drivers/fpga/zynqmppl.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c > index 239c498f7b5..6959b8ae97e 100644 > --- a/drivers/fpga/zynqmppl.c > +++ b/drivers/fpga/zynqmppl.c > @@ -199,46 +199,45 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf, > return 0; > } > > -static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, > - bitstream_type bstype, int flags) > +static int zynqmp_load(xilinx_desc *desc, const void *buf, > + size_t bsize, bitstream_type bstype, > + int flags) This is unrelated to commit. This is purely coding style change. > { > ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1); > u32 swap = 0; > ulong bin_buf; > int ret; > u32 buf_lo, buf_hi; > + u32 bsize_req = (u32)bsize; > u32 ret_payload[PAYLOAD_ARG_CNT]; > - bool xilfpga_old = false; > + > + debug("%s called!\n", __func__); > > if (zynqmp_firmware_version() <= PMUFW_V1_0) { > puts("WARN: PMUFW v1.0 or less is detected\n"); > puts("WARN: Not all bitstream formats are supported\n"); > puts("WARN: Please upgrade PMUFW\n"); > - xilfpga_old = true; > - if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap)) > + if (zynqmp_validate_bitstream(desc, buf, bsize, > + bsize, &swap)) This is also coding style change only. > return FPGA_FAIL; > bsizeptr = (u32 *)&bsize; > flush_dcache_range((ulong)bsizeptr, > (ulong)bsizeptr + sizeof(size_t)); > + bsize_req = (u32)(uintptr_t)bsizeptr; > bstype |= BIT(ZYNQMP_FPGA_BIT_NS); > + } else { > + bstype = 0; > } > > bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap); > > - debug("%s called!\n", __func__); nit: And this also has nothing to do with optimization. You just changed location. > flush_dcache_range(bin_buf, bin_buf + bsize); > > buf_lo = (u32)bin_buf; > buf_hi = upper_32_bits(bin_buf); > > - if (xilfpga_old) > - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, > - buf_hi, (u32)(uintptr_t)bsizeptr, > - bstype, ret_payload); > - else > - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, > - buf_hi, (u32)bsize, 0, ret_payload); > - > + ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, buf_hi, > + bsize_req, bstype, ret_payload); > if (ret) > printf("PL FPGA LOAD failed with err: 0x%08x\n", ret); > M
Hi Michal, On Fri, Jul 8, 2022 at 3:43 PM Michal Simek <michal.simek@amd.com> wrote: > > > > On 7/5/22 21:23, Oleksandr Suvorov wrote: > > Optimize function code preparing to add secure bitstream types > > support. > > Can you please extend this? I understand what you do below but better > description will be good. Ok, if I'll realize how to do this :) > > > > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io> > > Tested-by: Ricardo Salveti <ricardo@foundries.io> > > Tested-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com> > > --- > > > > (no changes since v1) > > > > drivers/fpga/zynqmppl.c | 27 +++++++++++++-------------- > > 1 file changed, 13 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c > > index 239c498f7b5..6959b8ae97e 100644 > > --- a/drivers/fpga/zynqmppl.c > > +++ b/drivers/fpga/zynqmppl.c > > @@ -199,46 +199,45 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf, > > return 0; > > } > > > > -static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, > > - bitstream_type bstype, int flags) > > +static int zynqmp_load(xilinx_desc *desc, const void *buf, > > + size_t bsize, bitstream_type bstype, > > + int flags) > > This is unrelated to commit. This is purely coding style change. Ok, I'll separate to another commit. > > { > > ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1); > > u32 swap = 0; > > ulong bin_buf; > > int ret; > > u32 buf_lo, buf_hi; > > + u32 bsize_req = (u32)bsize; > > u32 ret_payload[PAYLOAD_ARG_CNT]; > > - bool xilfpga_old = false; > > + > > + debug("%s called!\n", __func__); > > > > if (zynqmp_firmware_version() <= PMUFW_V1_0) { > > puts("WARN: PMUFW v1.0 or less is detected\n"); > > puts("WARN: Not all bitstream formats are supported\n"); > > puts("WARN: Please upgrade PMUFW\n"); > > - xilfpga_old = true; > > - if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap)) > > + if (zynqmp_validate_bitstream(desc, buf, bsize, > > + bsize, &swap)) > > This is also coding style change only. Ok. > > return FPGA_FAIL; > > bsizeptr = (u32 *)&bsize; > > flush_dcache_range((ulong)bsizeptr, > > (ulong)bsizeptr + sizeof(size_t)); > > + bsize_req = (u32)(uintptr_t)bsizeptr; > > bstype |= BIT(ZYNQMP_FPGA_BIT_NS); > > + } else { > > + bstype = 0; > > } > > > > bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap); > > > > - debug("%s called!\n", __func__); > > nit: And this also has nothing to do with optimization. You just changed location. Michal, what is this closer to? Refactor? It's not only about changing location. Now there is only one call of xilinx_pm_request(). > > flush_dcache_range(bin_buf, bin_buf + bsize); > > > > buf_lo = (u32)bin_buf; > > buf_hi = upper_32_bits(bin_buf); > > > > - if (xilfpga_old) > > - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, > > - buf_hi, (u32)(uintptr_t)bsizeptr, > > - bstype, ret_payload); > > - else > > - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, > > - buf_hi, (u32)bsize, 0, ret_payload); > > - > > + ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, buf_hi, > > + bsize_req, bstype, ret_payload); > > if (ret) > > printf("PL FPGA LOAD failed with err: 0x%08x\n", ret); > > > > M
On 7/8/22 15:15, Oleksandr Suvorov wrote: > Hi Michal, > > On Fri, Jul 8, 2022 at 3:43 PM Michal Simek <michal.simek@amd.com> wrote: >> >> >> >> On 7/5/22 21:23, Oleksandr Suvorov wrote: >>> Optimize function code preparing to add secure bitstream types >>> support. >> >> Can you please extend this? I understand what you do below but better >> description will be good. > > Ok, if I'll realize how to do this :) > >>> >>> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io> >>> Tested-by: Ricardo Salveti <ricardo@foundries.io> >>> Tested-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com> >>> --- >>> >>> (no changes since v1) >>> >>> drivers/fpga/zynqmppl.c | 27 +++++++++++++-------------- >>> 1 file changed, 13 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c >>> index 239c498f7b5..6959b8ae97e 100644 >>> --- a/drivers/fpga/zynqmppl.c >>> +++ b/drivers/fpga/zynqmppl.c >>> @@ -199,46 +199,45 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf, >>> return 0; >>> } >>> >>> -static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, >>> - bitstream_type bstype, int flags) >>> +static int zynqmp_load(xilinx_desc *desc, const void *buf, >>> + size_t bsize, bitstream_type bstype, >>> + int flags) >> >> This is unrelated to commit. This is purely coding style change. > > Ok, I'll separate to another commit. > >>> { >>> ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1); >>> u32 swap = 0; >>> ulong bin_buf; >>> int ret; >>> u32 buf_lo, buf_hi; >>> + u32 bsize_req = (u32)bsize; >>> u32 ret_payload[PAYLOAD_ARG_CNT]; >>> - bool xilfpga_old = false; >>> + >>> + debug("%s called!\n", __func__); >>> >>> if (zynqmp_firmware_version() <= PMUFW_V1_0) { >>> puts("WARN: PMUFW v1.0 or less is detected\n"); >>> puts("WARN: Not all bitstream formats are supported\n"); >>> puts("WARN: Please upgrade PMUFW\n"); >>> - xilfpga_old = true; >>> - if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap)) >>> + if (zynqmp_validate_bitstream(desc, buf, bsize, >>> + bsize, &swap)) >> >> This is also coding style change only. > > Ok. > >>> return FPGA_FAIL; >>> bsizeptr = (u32 *)&bsize; >>> flush_dcache_range((ulong)bsizeptr, >>> (ulong)bsizeptr + sizeof(size_t)); >>> + bsize_req = (u32)(uintptr_t)bsizeptr; >>> bstype |= BIT(ZYNQMP_FPGA_BIT_NS); >>> + } else { >>> + bstype = 0; >>> } >>> >>> bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap); >>> >>> - debug("%s called!\n", __func__); >> >> nit: And this also has nothing to do with optimization. You just changed location. > > Michal, what is this closer to? Refactor? > It's not only about changing location. Now there is only one call of > xilinx_pm_request(). I know but it s not described in commit message. Diff was really just passing different argument. You just change the logic about them where you use clear bstype for secure bitstreams and align bsize_req But you are not changing logic around bsizeptr where old xilfpga expects size to be a pointer not value. Thanks, Michal commit 31bcb3444cbd5002ca9d8f6a3a2644092748cdba Author: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com> AuthorDate: Thu Mar 15 00:17:24 2018 +0530 Commit: Michal Simek <michal.simek@amd.com> CommitDate: Mon Apr 9 12:14:50 2018 +0200 fpga: zynqmp: Fix the nonsecure bitstream loading issue Xilfpga library expects the size of bitstream in a pointer but currenly we are passing the size as a value. This patch fixes this issue. Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com> Signed-off-by: Nava kishore Manne <navam@xilinx.com> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c index 239c498f7b5..6959b8ae97e 100644 --- a/drivers/fpga/zynqmppl.c +++ b/drivers/fpga/zynqmppl.c @@ -199,46 +199,45 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf, return 0; } -static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, - bitstream_type bstype, int flags) +static int zynqmp_load(xilinx_desc *desc, const void *buf, + size_t bsize, bitstream_type bstype, + int flags) { ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1); u32 swap = 0; ulong bin_buf; int ret; u32 buf_lo, buf_hi; + u32 bsize_req = (u32)bsize; u32 ret_payload[PAYLOAD_ARG_CNT]; - bool xilfpga_old = false; + + debug("%s called!\n", __func__); if (zynqmp_firmware_version() <= PMUFW_V1_0) { puts("WARN: PMUFW v1.0 or less is detected\n"); puts("WARN: Not all bitstream formats are supported\n"); puts("WARN: Please upgrade PMUFW\n"); - xilfpga_old = true; - if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap)) + if (zynqmp_validate_bitstream(desc, buf, bsize, + bsize, &swap)) return FPGA_FAIL; bsizeptr = (u32 *)&bsize; flush_dcache_range((ulong)bsizeptr, (ulong)bsizeptr + sizeof(size_t)); + bsize_req = (u32)(uintptr_t)bsizeptr; bstype |= BIT(ZYNQMP_FPGA_BIT_NS); + } else { + bstype = 0; } bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap); - debug("%s called!\n", __func__); flush_dcache_range(bin_buf, bin_buf + bsize); buf_lo = (u32)bin_buf; buf_hi = upper_32_bits(bin_buf); - if (xilfpga_old) - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, - buf_hi, (u32)(uintptr_t)bsizeptr, - bstype, ret_payload); - else - ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, - buf_hi, (u32)bsize, 0, ret_payload); - + ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, buf_hi, + bsize_req, bstype, ret_payload); if (ret) printf("PL FPGA LOAD failed with err: 0x%08x\n", ret);