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

[Abdellatif El Khlifi]

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