diff mbox series

[01/10] s390/cio: Export information about Endpoint-Security Capability

Message ID 20201002193940.24012-2-sth@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series DASD FC endpoint security | expand

Commit Message

Stefan Haberland Oct. 2, 2020, 7:39 p.m. UTC 
From: Sebastian Ott <sebott@linux.ibm.com>

Add a new sysfs attribute 'esc' per chpid. This new attribute exports
the Endpoint-Security-Capability byte of channel-path description block,
which could be 0-None, 1-Authentication, 2 and 3-Encryption.

For example:
$ cat /sys/devices/css0/chp0.34/esc
0

Reference-ID: IO1812
Signed-off-by: Sebastian Ott <sebott@linux.ibm.com>
[vneethv@linux.ibm.com: cleaned-up & modified description]
Signed-off-by: Vineeth Vijayan <vneethv@linux.ibm.com>
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
Acked-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
---
 drivers/s390/cio/chp.c  | 15 +++++++++++++++
 drivers/s390/cio/chsc.h |  3 ++-
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Cornelia Huck Oct. 6, 2020, 9:46 a.m. UTC | #1 
On Fri,  2 Oct 2020 21:39:31 +0200
Stefan Haberland <sth@linux.ibm.com> wrote:

> From: Sebastian Ott <sebott@linux.ibm.com>
> 
> Add a new sysfs attribute 'esc' per chpid. This new attribute exports
> the Endpoint-Security-Capability byte of channel-path description block,
> which could be 0-None, 1-Authentication, 2 and 3-Encryption.
> 
> For example:
> $ cat /sys/devices/css0/chp0.34/esc
> 0
> 
> Reference-ID: IO1812
> Signed-off-by: Sebastian Ott <sebott@linux.ibm.com>
> [vneethv@linux.ibm.com: cleaned-up & modified description]
> Signed-off-by: Vineeth Vijayan <vneethv@linux.ibm.com>
> Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
> Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
> Acked-by: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
> ---
>  drivers/s390/cio/chp.c  | 15 +++++++++++++++
>  drivers/s390/cio/chsc.h |  3 ++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
> index dfcbe54591fb..8d0de6adcad0 100644
> --- a/drivers/s390/cio/chp.c
> +++ b/drivers/s390/cio/chp.c
> @@ -384,6 +384,20 @@ static ssize_t chp_chid_external_show(struct device *dev,
>  }
>  static DEVICE_ATTR(chid_external, 0444, chp_chid_external_show, NULL);
>  
> +static ssize_t chp_esc_show(struct device *dev,
> +			    struct device_attribute *attr, char *buf)
> +{
> +	struct channel_path *chp = to_channelpath(dev);
> +	ssize_t rc;
> +
> +	mutex_lock(&chp->lock);
> +	rc = sprintf(buf, "%x\n", chp->desc_fmt1.esc);

I'm wondering: Do we need to distinguish between '0' == 'no esc, and
the hardware says so' and '0' == 'the chsc to get that information is
not supported'? I see that for the chid the code checks for a flag in
desc_fmt1, and I indeed see that nothing is displayed for
chid/chid_external when I run under QEMU.

> +	mutex_unlock(&chp->lock);
> +
> +	return rc;
> +}
> +static DEVICE_ATTR(esc, 0444, chp_esc_show, NULL);
> +
>  static ssize_t util_string_read(struct file *filp, struct kobject *kobj,
>  				struct bin_attribute *attr, char *buf,
>  				loff_t off, size_t count)

(...)
Stefan Haberland Oct. 6, 2020, 2:23 p.m. UTC | #2 
Hi,

talked to Vineeth, here is his answer...

Am 06.10.20 um 11:46 schrieb Cornelia Huck:
> On Fri,  2 Oct 2020 21:39:31 +0200
> Stefan Haberland <sth@linux.ibm.com> wrote:
>
>> From: Sebastian Ott <sebott@linux.ibm.com>
>>
>> Add a new sysfs attribute 'esc' per chpid. This new attribute exports
>> the Endpoint-Security-Capability byte of channel-path description block,
>> which could be 0-None, 1-Authentication, 2 and 3-Encryption.
>>
>> For example:
>> $ cat /sys/devices/css0/chp0.34/esc
>> 0
>>
>> Reference-ID: IO1812
>> Signed-off-by: Sebastian Ott <sebott@linux.ibm.com>
>> [vneethv@linux.ibm.com: cleaned-up & modified description]
>> Signed-off-by: Vineeth Vijayan <vneethv@linux.ibm.com>
>> Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
>> Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
>> Acked-by: Vasily Gorbik <gor@linux.ibm.com>
>> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
>> ---
>>  drivers/s390/cio/chp.c  | 15 +++++++++++++++
>>  drivers/s390/cio/chsc.h |  3 ++-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
>> index dfcbe54591fb..8d0de6adcad0 100644
>> --- a/drivers/s390/cio/chp.c
>> +++ b/drivers/s390/cio/chp.c
>> @@ -384,6 +384,20 @@ static ssize_t chp_chid_external_show(struct device *dev,
>>  }
>>  static DEVICE_ATTR(chid_external, 0444, chp_chid_external_show, NULL);
>>  
>> +static ssize_t chp_esc_show(struct device *dev,
>> +			    struct device_attribute *attr, char *buf)
>> +{
>> +	struct channel_path *chp = to_channelpath(dev);
>> +	ssize_t rc;
>> +
>> +	mutex_lock(&chp->lock);
>> +	rc = sprintf(buf, "%x\n", chp->desc_fmt1.esc);
> I'm wondering: Do we need to distinguish between '0' == 'no esc, and
> the hardware says so' and '0' == 'the chsc to get that information is
> not supported'? I see that for the chid the code checks for a flag in
> desc_fmt1, and I indeed see that nothing is displayed for
> chid/chid_external when I run under QEMU.

ESC==0 due to 'missing support for the required CHSC information' is
just another symptom of "unsupported" because the CSS firmware code
doesn't bring the required support.
Also, not sure if there is any flag/value which provide this
distinction. So we think having 2 different values "Unknown" and
"Unsupported" is not required in this scenario.

So, we kept a single "ESC==0" which indicates "Unsupported", but as you
mentioned, in different levels.

>> +	mutex_unlock(&chp->lock);
>> +
>> +	return rc;
>> +}
>> +static DEVICE_ATTR(esc, 0444, chp_esc_show, NULL);
>> +
>>  static ssize_t util_string_read(struct file *filp, struct kobject *kobj,
>>  				struct bin_attribute *attr, char *buf,
>>  				loff_t off, size_t count)
> (...)
>
Cornelia Huck Oct. 6, 2020, 2:37 p.m. UTC | #3 
On Tue, 6 Oct 2020 16:23:36 +0200
Stefan Haberland <sth@linux.ibm.com> wrote:

> Hi,
> 
> talked to Vineeth, here is his answer...
> 
> Am 06.10.20 um 11:46 schrieb Cornelia Huck:
> > On Fri,  2 Oct 2020 21:39:31 +0200
> > Stefan Haberland <sth@linux.ibm.com> wrote:
> >  
> >> From: Sebastian Ott <sebott@linux.ibm.com>
> >>
> >> Add a new sysfs attribute 'esc' per chpid. This new attribute exports
> >> the Endpoint-Security-Capability byte of channel-path description block,
> >> which could be 0-None, 1-Authentication, 2 and 3-Encryption.
> >>
> >> For example:
> >> $ cat /sys/devices/css0/chp0.34/esc
> >> 0
> >>
> >> Reference-ID: IO1812
> >> Signed-off-by: Sebastian Ott <sebott@linux.ibm.com>
> >> [vneethv@linux.ibm.com: cleaned-up & modified description]
> >> Signed-off-by: Vineeth Vijayan <vneethv@linux.ibm.com>
> >> Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
> >> Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
> >> Acked-by: Vasily Gorbik <gor@linux.ibm.com>
> >> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
> >> ---
> >>  drivers/s390/cio/chp.c  | 15 +++++++++++++++
> >>  drivers/s390/cio/chsc.h |  3 ++-
> >>  2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
> >> index dfcbe54591fb..8d0de6adcad0 100644
> >> --- a/drivers/s390/cio/chp.c
> >> +++ b/drivers/s390/cio/chp.c
> >> @@ -384,6 +384,20 @@ static ssize_t chp_chid_external_show(struct device *dev,
> >>  }
> >>  static DEVICE_ATTR(chid_external, 0444, chp_chid_external_show, NULL);
> >>  
> >> +static ssize_t chp_esc_show(struct device *dev,
> >> +			    struct device_attribute *attr, char *buf)
> >> +{
> >> +	struct channel_path *chp = to_channelpath(dev);
> >> +	ssize_t rc;
> >> +
> >> +	mutex_lock(&chp->lock);
> >> +	rc = sprintf(buf, "%x\n", chp->desc_fmt1.esc);  
> > I'm wondering: Do we need to distinguish between '0' == 'no esc, and
> > the hardware says so' and '0' == 'the chsc to get that information is
> > not supported'? I see that for the chid the code checks for a flag in
> > desc_fmt1, and I indeed see that nothing is displayed for
> > chid/chid_external when I run under QEMU.  
> 
> ESC==0 due to 'missing support for the required CHSC information' is
> just another symptom of "unsupported" because the CSS firmware code
> doesn't bring the required support.
> Also, not sure if there is any flag/value which provide this
> distinction. So we think having 2 different values "Unknown" and
> "Unsupported" is not required in this scenario.
> 
> So, we kept a single "ESC==0" which indicates "Unsupported", but as you
> mentioned, in different levels.

Ok, that makes sense, also considering how this is used later on.

> 
> >> +	mutex_unlock(&chp->lock);
> >> +
> >> +	return rc;
> >> +}
> >> +static DEVICE_ATTR(esc, 0444, chp_esc_show, NULL);
> >> +
> >>  static ssize_t util_string_read(struct file *filp, struct kobject *kobj,
> >>  				struct bin_attribute *attr, char *buf,
> >>  				loff_t off, size_t count)  
> > (...)
> >  
> 
> 

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff mbox series

Patch

diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
index dfcbe54591fb..8d0de6adcad0 100644
--- a/drivers/s390/cio/chp.c
+++ b/drivers/s390/cio/chp.c
@@ -384,6 +384,20 @@  static ssize_t chp_chid_external_show(struct device *dev,
 }
 static DEVICE_ATTR(chid_external, 0444, chp_chid_external_show, NULL);
 
+static ssize_t chp_esc_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct channel_path *chp = to_channelpath(dev);
+	ssize_t rc;
+
+	mutex_lock(&chp->lock);
+	rc = sprintf(buf, "%x\n", chp->desc_fmt1.esc);
+	mutex_unlock(&chp->lock);
+
+	return rc;
+}
+static DEVICE_ATTR(esc, 0444, chp_esc_show, NULL);
+
 static ssize_t util_string_read(struct file *filp, struct kobject *kobj,
 				struct bin_attribute *attr, char *buf,
 				loff_t off, size_t count)
@@ -414,6 +428,7 @@  static struct attribute *chp_attrs[] = {
 	&dev_attr_shared.attr,
 	&dev_attr_chid.attr,
 	&dev_attr_chid_external.attr,
+	&dev_attr_esc.attr,
 	NULL,
 };
 static struct attribute_group chp_attr_group = {
diff --git a/drivers/s390/cio/chsc.h b/drivers/s390/cio/chsc.h
index 7ecf7e4c402e..4f049d17355d 100644
--- a/drivers/s390/cio/chsc.h
+++ b/drivers/s390/cio/chsc.h
@@ -27,7 +27,8 @@  struct channel_path_desc_fmt1 {
 	u8 lsn;
 	u8 desc;
 	u8 chpid;
-	u32:24;
+	u32:16;
+	u8 esc;
 	u8 chpp;
 	u32 unused[2];
 	u16 chid;