Re: [PATCH v13 05/10] arm_ffa: introduce armffa command

Hi Ilias,

On Tue, Jun 20, 2023 at 05:25:51PM +0300, Ilias Apalodimas wrote:

[...]

...

+int do_ffa_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{

•   struct ffa_send_direct_data msg = {
  

•                   .data0 = 0xaaaaaaaa,
  

•                   .data1 = 0xbbbbbbbb,
  

•                   .data2 = 0xcccccccc,
  

•                   .data3 = 0xdddddddd,
  

•                   .data4 = 0xeeeeeeee,
  

•   };
  

•   u16 part_id;
  

•   int ret;
  

•   struct udevice *dev;
  

•   if (argc != 2) {
  

•           log_err("Missing argument\n");
  

•           return CMD_RET_USAGE;
  

•   }
  

•   errno = 0;
  

•   part_id = strtoul(argv[1], NULL, 16);
  

•   if (errno) {
  

Is errno used in strtoul?

Yes, please refer to [1].

[1]: https://man7.org/linux/man-pages/man3/strtoul.3.html

Does FF-A have any limits regarding the partition id? If yes, it would be saner to check against that.

The only value that would be invalid is 0. I'll add a check for that in v14.

...

•           log_err("Invalid partition ID\n");
  

•           return CMD_RET_USAGE;
  

•   }
  

•   ret = ffa_get_dev(&dev);
  

•   if (ret)
  

•           return CMD_RET_FAILURE;
  

•   ret = ffa_sync_send_receive(dev, part_id, &msg, 1);
  

•   if (!ret) {
  

•           u8 cnt;
  

•           log_info("SP response:\n[LSB]\n");
  

•           for (cnt = 0;
  

•                cnt < sizeof(struct ffa_send_direct_data) / sizeof(u64);
  

•                cnt++)
  

•                   log_info("%llx\n", ((u64 *)&msg)[cnt]);
  

I am not sure I understand why we print it like this.

We would like to show the data received from secure world and in which order.

example:

corstone1000# armffa ping 0x8003 SP response: [LSB] fffffffe 0 0 0 0

...

•           return CMD_RET_SUCCESS;
  

•   }
  

•   log_err("Sending direct request error (%d)\n", ret);
  

•   return CMD_RET_FAILURE;
  

+}

+/**

• *do_ffa_devlist() - implementation of the devlist subcommand
• • @cmdtp: [in] Command Table
• • @flag: flags
• • @argc: number of arguments
• • @argv: arguments
• • Query the device belonging to the UCLASS_FFA
• • class.
• • Return:
• • CMD_RET_SUCCESS: on success, otherwise failure
• */

+int do_ffa_devlist(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{

•   struct udevice *dev;
  

•   int ret;
  

•   ret = ffa_get_dev(&dev);
  

•   if (ret)
  

•           return CMD_RET_FAILURE;
  

•   log_info("device name %s, dev %p, driver name %s, ops %p\n",
  

•            dev->name,
  

•           (void *)map_to_sysmem(dev),
  

•            dev->driver->name,
  

•            (void *)map_to_sysmem(dev->driver->ops));
  

Isn't it more useful to print the physical address map_to_sysmem() retuns?

That's what map_to_sysmem() does, it returns a physical address and it's shown in the log.

Cheers, Abdellatif