(Dangit! My email client crashed and I lost the response I was typing!) @mpe I'd still like it if you weighed in on this one. It's looking pretty good to me now aside from one pointer used in structure passed to the new ioctl (and once that is addressed you may consider this Acked-By: me), but since you had some feedback on our original user API I'd appreciate it if you could take a look at this one also.
Excerpts from Frederic Barrat's message of 2016-02-24 03:21:55 +1100: > + case VALIDATE_IMAGE: ... > + for (afu = 0; afu < adapter->slices; afu++) > + cxl_guest_remove_afu(adapter->afu[afu]); Thanks for moving that here - that should remove one possible race :) > diff --git a/include/uapi/misc/cxl.h b/include/uapi/misc/cxl.h Thanks for moving these to the correct header :) > +struct cxl_adapter_image { > + __u64 flags; > + __u64 *data; This isn't quite what I had in mind - using a * still makes that a pointer and will still have variable size depending on userspace, regardless of what size data it is pointing too. You could potentially handle the difference in the compat_ioctl, but that is more intended as a last resort to fix up broken APIs, not to rely on when adding a new one. I think it would be better to just declare that as a __u64 and have userspace cast to it so that the struct will always be consistent (similar to the WED for AFUs that define that as an address). > + __u64 len_data; > + __u64 len_image; > + __u64 reserved1; > + __u64 reserved2; > + __u64 reserved3; > + __u64 reserved4; Thanks for changing the rest of this structure to be more consistent with the other cxl user APIs & checking the flags+reserved fields in the code :) > +#define CXL_IOCTL_DOWNLOAD_IMAGE _IOW(CXL_MAGIC, 0x0A, struct > cxl_adapter_image) > +#define CXL_IOCTL_VALIDATE_IMAGE _IOW(CXL_MAGIC, 0x0B, struct > cxl_adapter_image) Thanks for splitting those operations into two separate ioctls. Cheers, -Ian _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev