Thanks for reviewing the patch Fred!!
Frederic Barrat <fbar...@linux.vnet.ibm.com> writes:
>> +
>> + if (afu->attr_afud.size > 0) {
>> + sysfs_attr_init(&afu->attr_afud.attr);
>> + afu->attr_afud.attr.name = "afu_desc";
>> + afu->attr_afud.attr.mode = 0444;
>
>
> So I had a long pause here, wondering if we are showing too much
> information to ANY user. Most of the content of the AFU descriptor is
> already world-readable through other sysfs attributes, the only ones
> troubling me are offset 0x50 and 0x58, ie. the page resolution interrupt
> handling. We currently don't support it (yet), but we need to think
> about it. A user process, by monitoring offset x50 could get a glimpse
> of the layout of the address space of other processes. Not the actual
> content, but at least what addresses are valid. I think that's already
> too much.
As we discussed "Paged Resolution EA (offset 0x58)" register is write
only should ideally return 0x00 for all reads (need to confirm with
h/w on this). Secondly there isnt much information exposed from register
"Paged Resolution Handle(offset 0x50)" apart from process handle. Lastly
presently Paged-Response Resolution isn't supported for cxl on linux. So,
I think we are safe for the time being.
> Why not just exporting an area only covering 64 bits (and guaranteed not
> to be all 1's) and call it mmio_check? It would seem safer to me.
AFAIK the minimum granuality that we have is PAGE_SIZE and we cannot mmap
anything less than PAGE_SIZE. Though one exception for hashed paged
table on power, we have an ability to map 4K PFN on 64K hosts but it
isnt supported for radix on P9.
At best I can limit access to first page of the afu descriptor.
--
Vaibhav Jain <vaib...@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
