ahci add "em_buffer" attribute for AHCI hosts
PATCH v4 1/2 ahci add "em_buffer" attribute for AHCI hosts
Add "em_buffer" attribute for SATA AHCI hosts to provide a way for userland to access AHCI EM (enclosure management) buffer directly if the host supports EM.
AHCI driver should support SGPIO EM messages. However the SATA/AHCI specs did not define the SGPIO message format filled in EM buffer. Different HW vendors may have different definitions. The mainly purpose of this attribute is to solve this issue by allowing HW vendors to provide userland drivers and tools for their SGPIO initiators.
---
v2: rebase this patch on the new upstream branch of linux-next.git
v3: do not use "ahci_em_messages" module parameter as EM message type
control, instead by detecting supported types at initialization. Remove EM message structure shared with userland, instead by simply writing the raw message into the buffer.
Move write function for this attribute from libata-scsi.c to libahci.c.
Add read function.
Add necessary definitions for EM in ahci.h.
v4: remove changes for EM message types.
add PAGE_SIZE check for em_buffer read function. report EAGAIN for read function when no message in the buffer.
drivers/ata/ahci.h | 10 +++- drivers/ata/libahci.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-)
drivers/ata/ahci.h +7 -3
@@ -224,9 +224,12 @@ enum {EM_MAX_RETRY =5 ,/* em_ctl bits */ -EM_CTL_RST = (1 <<9 ),/* Reset */ -EM_CTL_TM = (1 <<8 ),/* Transmit Message */ - EM_CTL_ALHD = (1 << 26), /* Activity LED */ +EM_CTL_RST = (1 <<9 ),/* Reset */ +EM_CTL_TM = (1 <<8 ),/* Transmit Message */ +EM_CTL_MR = (1 <<0 ),/* Message Recieved */ +EM_CTL_ALHD = (1 <<26 ),/* Activity LED */ +EM_CTL_XMT = (1 <<25 ),/* Transmit Only */ + EM_CTL_SMB = (1 << 24), /* Single Message Buffer */ }; struct ahci_cmd_hdr {
@@ -282,6 +285,7 @@ struct ahci_host_priv {u32 saved_cap2;/* saved initial cap2 */ u32 saved_port_map;/* saved initial port_map */ u32 em_loc;/* enclosure management location */ +u32 em_buf_sz;/* EM buffer size in byte */ };extern int ahci_em_messages;
drivers/ata/libahci.c +103 -0
132 lines not on this page · raw message
Signature · 1 line
1.6.4.2
Hello, Harry.
drivers/ata/libahci.c static ssize_t ahci_show_port_cmd(struct device *dev, · line 297
Quoted 4 lines · Harry Zhang
drivers/ata/libahci.c +4
-0+/* Since EM buffer is in ABAR, commonly, the buffer size should be +* less than a page. Check buffer size against PAGE_SIZE in case of +* some rare instance. Only transfer the first page in this case. +*/
Oh, the PAGE_SIZE limit comes from the way sysfs attributes are implemented. The kernel buffer sysfs uses is PAGE_SIZE so reads/writes can't be larger than that. If you write past PAGE_SIZE from show, you'll corrupt someone else's memory.
drivers/ata/libahci.c static ssize_t ahci_show_port_cmd(struct device *dev, · line 303
Quoted 6 lines
drivers/ata/libahci.c +6
-0+if (count >PAGE_SIZE ) { +dev_printk (KERN_WARNING , dev, +"EM read buffer size %u is larger than %lu" , + hpriv->em_buf_sz,PAGE_SIZE ); + count =PAGE_SIZE ; + }
It probably would be better to use ata_port_printk() and printk_ratelimit() the message.
Thanks.
Signature · 1 line
tejun
Hi Tejun,
Quoted 12 lines · Tejun Heo
Hello, Harry.
drivers/ata/libahci.c static ssize_t ahci_show_port_cmd(struct device *dev, · line 297
Quoted 4 lines · Harry Zhang
drivers/ata/libahci.c +4
-0+/* Since EM buffer is in ABAR, commonly, the buffer size should be +* less than a page. Check buffer size against PAGE_SIZE in case of +* some rare instance. Only transfer the first page in this case. +*/ Oh, the
PAGE_SIZElimit comes from the way sysfs attributes are implemented. The kernel buffer sysfs uses isPAGE_SIZEso reads/writes can't be larger than that. If you write pastPAGE_SIZEfrom show, you'll corrupt someone else's memory.
Yes, I know that. I just think the EM read buffer size should not larger than the PAGE_SIZE in common, and thus, should not break the sysfs attributes r/w buffer limitation. Anyway, I will shorten the comment.
Quoted 10 lines
drivers/ata/libahci.c static ssize_t ahci_show_port_cmd(struct device *dev, · line 303
Quoted 7 lines
drivers/ata/libahci.c +6
-0+if (count >PAGE_SIZE ) { +dev_printk (KERN_WARNING , dev, +"EM read buffer size %u is larger than %lu" , + hpriv->em_buf_sz,PAGE_SIZE ); + count =PAGE_SIZE ; + }It probably would be better to use
ata_port_printk()andprintk_ratelimit()the message.
OK. I could not determine which is better. I think the EM buffer is belong to the host rather than a port, so I chose the "dev_printk". BTW, should this be a warning or an error?
Quoted 2 lines
Thanks.
Thanks,
Harry
Hello,
Quoted 7 lines · Harry Zhang
Quoted 4 lines
Oh, the
PAGE_SIZElimit comes from the way sysfs attributes are implemented. The kernel buffer sysfs uses isPAGE_SIZEso reads/writes can't be larger than that. If you write pastPAGE_SIZEfrom show, you'll corrupt someone else's memory.Yes, I know that. I just think the EM read buffer size should not larger than the
PAGE_SIZEin common, and thus, should not break the sysfs attributes r/w buffer limitation. Anyway, I will shorten the comment.
Oh I see.
Quoted 6 lines
Quoted 2 lines
It probably would be better to use
ata_port_printk()andprintk_ratelimit()the message.OK. I could not determine which is better. I think the EM buffer is belong to the host rather than a port, so I chose the "
dev_printk". BTW, should this be a warning or an error?
The buffer is per ATA port, so I think it would be better to use ata_port_printk(). Hmmm... as the read will succeed anyway with truncated message body, maybe warning is better?
Thanks.
Signature · 1 line
tejun