[for-next,v2] IB/hfi1: Add debugfs to control expansion ROM write protect

Commit Message

Dennis Dalessandro April 11, 2019, 2:07 p.m. UTC

From: Josh Collier <[e-mail]>

Some kernels now enable CONFIG_IO_STRICT_DEVMEM
which prevents multiple handles to PCI resource0. In order
to continue to support expansion ROM updates while the
driver is loaded, the driver must now provide an interface
to control the expansion ROM write protection.

This patch adds an exprom_wp debugfs interface that
allows the hfi1_eprom user tool to disable the expansion ROM
write protection by opening the file and writing a '1'.
The write protection is released when writing a '0' or
automatically re-enabled when the handle is closed.
The current implementation will only allow one handle
to be opened at a time across all hfi1 devices.

Reviewed-by: Dennis Dalessandro <[e-mail]>
Signed-off-by: Josh Collier <[e-mail]>
Signed-off-by: Dennis Dalessandro <[e-mail]>

---
Changes since v1:
Use bool instead of int for disabled flag
Simplify open routine with test/set vs atomic_inc/dec.
---
 drivers/infiniband/hw/hfi1/debugfs.c |   74 ++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)

Comments

Leon Romanovsky April 12, 2019, 3:38 p.m. UTC | #1

On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> From: Josh Collier <[e-mail]>
>
> Some kernels now enable CONFIG_IO_STRICT_DEVMEM
> which prevents multiple handles to PCI resource0. In order
> to continue to support expansion ROM updates while the
> driver is loaded, the driver must now provide an interface
> to control the expansion ROM write protection.

From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
added to prevent this type of access, doesn't it?

>
> This patch adds an exprom_wp debugfs interface that
> allows the hfi1_eprom user tool to disable the expansion ROM
> write protection by opening the file and writing a '1'.
> The write protection is released when writing a '0' or
> automatically re-enabled when the handle is closed.
> The current implementation will only allow one handle
> to be opened at a time across all hfi1 devices.
>
> Reviewed-by: Dennis Dalessandro <[e-mail]>
> Signed-off-by: Josh Collier <[e-mail]>
> Signed-off-by: Dennis Dalessandro <[e-mail]>
>
> ---
> Changes since v1:
> Use bool instead of int for disabled flag
> Simplify open routine with test/set vs atomic_inc/dec.
> ---
>  drivers/infiniband/hw/hfi1/debugfs.c |   74 ++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
> index 057bb82..15efb4a 100644
> --- a/drivers/infiniband/hw/hfi1/debugfs.c
> +++ b/drivers/infiniband/hw/hfi1/debugfs.c
> @@ -1080,6 +1080,77 @@ static int qsfp2_debugfs_release(struct inode *in, struct file *fp)
>  	return __qsfp_debugfs_release(in, fp, 1);
>  }
>
> +#define EXPROM_WRITE_ENABLE BIT_ULL(14)
> +
> +static bool exprom_wp_disabled;
> +
> +static int exprom_wp_set(struct hfi1_devdata *dd, bool disable)
> +{
> +	u64 gpio_val = 0;
> +
> +	if (disable) {
> +		gpio_val = EXPROM_WRITE_ENABLE;
> +		exprom_wp_disabled = true;
> +		dd_dev_info(dd, "Disable Expansion ROM Write Protection\n");
> +	} else {
> +		exprom_wp_disabled = false;
> +		dd_dev_info(dd, "Enable Expansion ROM Write Protection\n");
> +	}
> +
> +	write_csr(dd, ASIC_GPIO_OUT, gpio_val);
> +	write_csr(dd, ASIC_GPIO_OE, gpio_val);
> +
> +	return 0;
> +}
> +
> +static ssize_t exprom_wp_debugfs_read(struct file *file, char __user *buf,
> +				      size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static ssize_t exprom_wp_debugfs_write(struct file *file,
> +				       const char __user *buf, size_t count,
> +				       loff_t *ppos)
> +{
> +	struct hfi1_pportdata *ppd = private2ppd(file);
> +	char cdata;
> +
> +	if (count != 1)
> +		return -EINVAL;
> +	if (get_user(cdata, buf))
> +		return -EFAULT;
> +	if (cdata == '0')
> +		exprom_wp_set(ppd->dd, false);
> +	else if (cdata == '1')
> +		exprom_wp_set(ppd->dd, true);
> +	else
> +		return -EINVAL;
> +
> +	return 1;
> +}
> +
> +static unsigned long exprom_in_use;
> +
> +static int exprom_wp_debugfs_open(struct inode *in, struct file *fp)
> +{
> +	if (test_and_set_bit(0, &exprom_in_use))
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +
> +static int exprom_wp_debugfs_release(struct inode *in, struct file *fp)
> +{
> +	struct hfi1_pportdata *ppd = private2ppd(fp);
> +
> +	if (exprom_wp_disabled)
> +		exprom_wp_set(ppd->dd, false);
> +	clear_bit(0, &exprom_in_use);
> +
> +	return 0;
> +}
> +
>  #define DEBUGFS_OPS(nm, readroutine, writeroutine)	\
>  { \
>  	.name = nm, \
> @@ -1119,6 +1190,9 @@ static int qsfp2_debugfs_release(struct inode *in, struct file *fp)
>  		     qsfp1_debugfs_open, qsfp1_debugfs_release),
>  	DEBUGFS_XOPS("qsfp2", qsfp2_debugfs_read, qsfp2_debugfs_write,
>  		     qsfp2_debugfs_open, qsfp2_debugfs_release),
> +	DEBUGFS_XOPS("exprom_wp", exprom_wp_debugfs_read,
> +		     exprom_wp_debugfs_write, exprom_wp_debugfs_open,
> +		     exprom_wp_debugfs_release),
>  	DEBUGFS_OPS("asic_flags", asic_flags_read, asic_flags_write),
>  	DEBUGFS_OPS("dc8051_memory", dc8051_memory_read, NULL),
>  	DEBUGFS_OPS("lcb", debugfs_lcb_read, debugfs_lcb_write),
>

Dennis Dalessandro April 12, 2019, 4:34 p.m. UTC | #2

On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
>> From: Josh Collier <[e-mail]>
>>
>> Some kernels now enable CONFIG_IO_STRICT_DEVMEM
>> which prevents multiple handles to PCI resource0. In order
>> to continue to support expansion ROM updates while the
>> driver is loaded, the driver must now provide an interface
>> to control the expansion ROM write protection.
> 
>  From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
> added to prevent this type of access, doesn't it?

Due to the config option using the resource file is no longer an option 
for manipulating the EPROM. There are some accesses that we want to be 
able to make through the PCI config space which is not affected by the 
above config option. However access to that is protected by the HW. So 
this patch provides a safe interface to toggle that protection while 
still preventing userspace from free range access to memory regions 
claimed by the driver.

-Denny

Jason Gunthorpe April 12, 2019, 7:21 p.m. UTC | #3

On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
> On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> > On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> > > From: Josh Collier <[e-mail]>
> > > 
> > > Some kernels now enable CONFIG_IO_STRICT_DEVMEM
> > > which prevents multiple handles to PCI resource0. In order
> > > to continue to support expansion ROM updates while the
> > > driver is loaded, the driver must now provide an interface
> > > to control the expansion ROM write protection.
> > 
> >  From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
> > added to prevent this type of access, doesn't it?
> 
> Due to the config option using the resource file is no longer an option for
> manipulating the EPROM. There are some accesses that we want to be able to
> make through the PCI config space which is not affected by the above config
> option. However access to that is protected by the HW. So this patch
> provides a safe interface to toggle that protection while still preventing
> userspace from free range access to memory regions claimed by the driver.

I feel like this should require CAP_SYS_ADMIN or maybe
CAP_SYS_RAW_IO..

But I'm never sure when to check caps or when default permissions on
file nodes is good enough

Jason

Leon Romanovsky April 12, 2019, 7:53 p.m. UTC | #4

On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
> On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> > On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> > > From: Josh Collier <[e-mail]>
> > >
> > > Some kernels now enable CONFIG_IO_STRICT_DEVMEM
> > > which prevents multiple handles to PCI resource0. In order
> > > to continue to support expansion ROM updates while the
> > > driver is loaded, the driver must now provide an interface
> > > to control the expansion ROM write protection.
> >
> >  From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
> > added to prevent this type of access, doesn't it?
>
> Due to the config option using the resource file is no longer an option for
> manipulating the EPROM. There are some accesses that we want to be able to
> make through the PCI config space which is not affected by the above config
> option. However access to that is protected by the HW. So this patch
> provides a safe interface to toggle that protection while still preventing
> userspace from free range access to memory regions claimed by the driver.

Thanks for the explanation,

There is one knowledge gap which will be very good to understand.

Someone implemented security option, users decided that it is right thing
and enabled it. Why do you think that it is ok to "disable" such
security check in driver code?

Thanks

>
> -Denny

Dennis Dalessandro April 15, 2019, 12:23 p.m. UTC | #5

On 4/12/2019 3:21 PM, Jason Gunthorpe wrote:
> On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
>> On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
>>> On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
>>>> From: Josh Collier <[e-mail]>
>>>>
>>>> Some kernels now enable CONFIG_IO_STRICT_DEVMEM
>>>> which prevents multiple handles to PCI resource0. In order
>>>> to continue to support expansion ROM updates while the
>>>> driver is loaded, the driver must now provide an interface
>>>> to control the expansion ROM write protection.
>>>
>>>   From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
>>> added to prevent this type of access, doesn't it?
>>
>> Due to the config option using the resource file is no longer an option for
>> manipulating the EPROM. There are some accesses that we want to be able to
>> make through the PCI config space which is not affected by the above config
>> option. However access to that is protected by the HW. So this patch
>> provides a safe interface to toggle that protection while still preventing
>> userspace from free range access to memory regions claimed by the driver.
> 
> I feel like this should require CAP_SYS_ADMIN or maybe
> CAP_SYS_RAW_IO..
> 
> But I'm never sure when to check caps or when default permissions on
> file nodes is good enough

Fair point, we'll look into it a bit more.

-Denny

Collier, Josh D April 15, 2019, 9:20 p.m. UTC | #6

> -----Original Message-----
> From: Jason Gunthorpe [mailto:[e-mail]]
> Sent: Friday, April 12, 2019 3:22 PM
> To: Dalessandro, Dennis <[e-mail]>
> Cc: Leon Romanovsky <[e-mail]>; [e-mail]; linux-
> [e-mail]; Collier, Josh D <[e-mail]>
> Subject: Re: [PATCH for-next v2] IB/hfi1: Add debugfs to control expansion
> ROM write protect
> 
> On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
> > On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> > > On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> > > > From: Josh Collier <[e-mail]>
> > > >
> > > > Some kernels now enable CONFIG_IO_STRICT_DEVMEM which prevents
> > > > multiple handles to PCI resource0. In order to continue to support
> > > > expansion ROM updates while the driver is loaded, the driver must
> > > > now provide an interface to control the expansion ROM write
> > > > protection.
> > >
> > >  From this description, it seems like that CONFIG_IO_STRICT_DEVMEM
> > > was added to prevent this type of access, doesn't it?
> >
> > Due to the config option using the resource file is no longer an
> > option for manipulating the EPROM. There are some accesses that we
> > want to be able to make through the PCI config space which is not
> > affected by the above config option. However access to that is
> > protected by the HW. So this patch provides a safe interface to toggle
> > that protection while still preventing userspace from free range access to
> memory regions claimed by the driver.
> 
> I feel like this should require CAP_SYS_ADMIN or maybe CAP_SYS_RAW_IO..
> 
> But I'm never sure when to check caps or when default permissions on file
> nodes is good enough

By default debugfs is only root accessible. The only CAP_SYS_ADMIN references
in all of drivers/infiniband are in hfi1 for a sysfs related attribute and one
other non-debugfs behavior.

-Josh

> 
> Jason

Dennis Dalessandro April 22, 2019, 11:31 a.m. UTC | #7

Sorry for the delayed response, this got dumped into my spam folder, 
which frustratingly happens from time to time.

On 4/12/2019 3:53 PM, Leon Romanovsky wrote:
> On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
>> On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
>>> On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
>>>> From: Josh Collier <[e-mail]>
>>>>
>>>> Some kernels now enable CONFIG_IO_STRICT_DEVMEM
>>>> which prevents multiple handles to PCI resource0. In order
>>>> to continue to support expansion ROM updates while the
>>>> driver is loaded, the driver must now provide an interface
>>>> to control the expansion ROM write protection.
>>>
>>>   From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
>>> added to prevent this type of access, doesn't it?
>>
>> Due to the config option using the resource file is no longer an option for
>> manipulating the EPROM. There are some accesses that we want to be able to
>> make through the PCI config space which is not affected by the above config
>> option. However access to that is protected by the HW. So this patch
>> provides a safe interface to toggle that protection while still preventing
>> userspace from free range access to memory regions claimed by the driver.
> 
> Thanks for the explanation,
> 
> There is one knowledge gap which will be very good to understand.
> 
> Someone implemented security option, users decided that it is right thing
> and enabled it. Why do you think that it is ok to "disable" such
> security check in driver code?

Since it was implemented long ago (years) but only just now being 
adopted by users, where "users" here is really "distros", I'd say it's 
not that critical of a security option.

As to why it's OK to disable in the driver, generally speaking I would 
say that's up to the HW vendor. A HW vendor knows their HW and will have 
made a decision as to if it's safe for their particular HW or not.

Now in this specific case, we are not circumventing, or disabling the 
security check. All we are doing is allowing a controlled write to a HW 
register that turns on/off some HW feature. This happens all over the 
drivers of course.

-Denny

Leon Romanovsky April 22, 2019, 12:13 p.m. UTC | #8

On Mon, Apr 22, 2019 at 07:31:51AM -0400, Dennis Dalessandro wrote:
> Sorry for the delayed response, this got dumped into my spam folder,
> which frustratingly happens from time to time.
>
> On 4/12/2019 3:53 PM, Leon Romanovsky wrote:
> > On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
> > > On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> > > > On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> > > > > From: Josh Collier <[e-mail]>
> > > > >
> > > > > Some kernels now enable CONFIG_IO_STRICT_DEVMEM
> > > > > which prevents multiple handles to PCI resource0. In order
> > > > > to continue to support expansion ROM updates while the
> > > > > driver is loaded, the driver must now provide an interface
> > > > > to control the expansion ROM write protection.
> > > >
> > > >   From this description, it seems like that CONFIG_IO_STRICT_DEVMEM was
> > > > added to prevent this type of access, doesn't it?
> > >
> > > Due to the config option using the resource file is no longer an option for
> > > manipulating the EPROM. There are some accesses that we want to be able to
> > > make through the PCI config space which is not affected by the above config
> > > option. However access to that is protected by the HW. So this patch
> > > provides a safe interface to toggle that protection while still preventing
> > > userspace from free range access to memory regions claimed by the driver.
> >
> > Thanks for the explanation,
> >
> > There is one knowledge gap which will be very good to understand.
> >
> > Someone implemented security option, users decided that it is right thing
> > and enabled it. Why do you think that it is ok to "disable" such
> > security check in driver code?
>
> Since it was implemented long ago (years) but only just now being adopted
> by users, where "users" here is really "distros", I'd say it's not that
> critical of a security option.
>
> As to why it's OK to disable in the driver, generally speaking I would
> say that's up to the HW vendor. A HW vendor knows their HW and will have
> made a decision as to if it's safe for their particular HW or not.
>
> Now in this specific case, we are not circumventing, or disabling the
> security check. All we are doing is allowing a controlled write to a HW
> register that turns on/off some HW feature. This happens all over the
> drivers of course.

Thanks, I disagree with your position about HW vendors, many such vendors
have no clue what they are doing. Luckily enough, Intel is not such vendor.

Thanks

>
> -Denny
>
>
>
>
>
>
>

Jason Gunthorpe April 22, 2019, 5:22 p.m. UTC | #9

On Mon, Apr 15, 2019 at 09:20:25PM +0000, Collier, Josh D wrote:
> 
> > From: Jason Gunthorpe [mailto:[e-mail]]
> > Sent: Friday, April 12, 2019 3:22 PM
> > To: Dalessandro, Dennis <[e-mail]>
> > Cc: Leon Romanovsky <[e-mail]>; [e-mail]; linux-
> > [e-mail]; Collier, Josh D <[e-mail]>
> > Subject: Re: [PATCH for-next v2] IB/hfi1: Add debugfs to control expansion
> > ROM write protect
> > 
> > On Fri, Apr 12, 2019 at 12:34:31PM -0400, Dennis Dalessandro wrote:
> > > On 4/12/2019 11:38 AM, Leon Romanovsky wrote:
> > > > On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> > > > > From: Josh Collier <[e-mail]>
> > > > >
> > > > > Some kernels now enable CONFIG_IO_STRICT_DEVMEM which prevents
> > > > > multiple handles to PCI resource0. In order to continue to support
> > > > > expansion ROM updates while the driver is loaded, the driver must
> > > > > now provide an interface to control the expansion ROM write
> > > > > protection.
> > > >
> > > >  From this description, it seems like that CONFIG_IO_STRICT_DEVMEM
> > > > was added to prevent this type of access, doesn't it?
> > >
> > > Due to the config option using the resource file is no longer an
> > > option for manipulating the EPROM. There are some accesses that we
> > > want to be able to make through the PCI config space which is not
> > > affected by the above config option. However access to that is
> > > protected by the HW. So this patch provides a safe interface to toggle
> > > that protection while still preventing userspace from free range access to
> > memory regions claimed by the driver.
> > 
> > I feel like this should require CAP_SYS_ADMIN or maybe CAP_SYS_RAW_IO..
> > 
> > But I'm never sure when to check caps or when default permissions on file
> > nodes is good enough
> 
> By default debugfs is only root accessible. The only CAP_SYS_ADMIN references
> in all of drivers/infiniband are in hfi1 for a sysfs related attribute and one
> other non-debugfs behavior.

Okay

Jason

Jason Gunthorpe April 24, 2019, 2:30 p.m. UTC | #10

On Thu, Apr 11, 2019 at 07:07:42AM -0700, Dennis Dalessandro wrote:
> From: Josh Collier <[e-mail]>
> 
> Some kernels now enable CONFIG_IO_STRICT_DEVMEM
> which prevents multiple handles to PCI resource0. In order
> to continue to support expansion ROM updates while the
> driver is loaded, the driver must now provide an interface
> to control the expansion ROM write protection.
> 
> This patch adds an exprom_wp debugfs interface that
> allows the hfi1_eprom user tool to disable the expansion ROM
> write protection by opening the file and writing a '1'.
> The write protection is released when writing a '0' or
> automatically re-enabled when the handle is closed.
> The current implementation will only allow one handle
> to be opened at a time across all hfi1 devices.
> 
> Reviewed-by: Dennis Dalessandro <[e-mail]>
> Signed-off-by: Josh Collier <[e-mail]>
> Signed-off-by: Dennis Dalessandro <[e-mail]>
> ---
> Changes since v1:
> Use bool instead of int for disabled flag
> Simplify open routine with test/set vs atomic_inc/dec.
> ---
>  drivers/infiniband/hw/hfi1/debugfs.c |   74 ++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)

Applied to for-next, thanks

Jason

Patch

diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 057bb82..15efb4a 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -1080,6 +1080,77 @@  static int qsfp2_debugfs_release(struct inode *in, struct file *fp)
 	return __qsfp_debugfs_release(in, fp, 1);
 }
 
+#define EXPROM_WRITE_ENABLE BIT_ULL(14)
+
+static bool exprom_wp_disabled;
+
+static int exprom_wp_set(struct hfi1_devdata *dd, bool disable)
+{
+	u64 gpio_val = 0;
+
+	if (disable) {
+		gpio_val = EXPROM_WRITE_ENABLE;
+		exprom_wp_disabled = true;
+		dd_dev_info(dd, "Disable Expansion ROM Write Protection\n");
+	} else {
+		exprom_wp_disabled = false;
+		dd_dev_info(dd, "Enable Expansion ROM Write Protection\n");
+	}
+
+	write_csr(dd, ASIC_GPIO_OUT, gpio_val);
+	write_csr(dd, ASIC_GPIO_OE, gpio_val);
+
+	return 0;
+}
+
+static ssize_t exprom_wp_debugfs_read(struct file *file, char __user *buf,
+				      size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static ssize_t exprom_wp_debugfs_write(struct file *file,
+				       const char __user *buf, size_t count,
+				       loff_t *ppos)
+{
+	struct hfi1_pportdata *ppd = private2ppd(file);
+	char cdata;
+
+	if (count != 1)
+		return -EINVAL;
+	if (get_user(cdata, buf))
+		return -EFAULT;
+	if (cdata == '0')
+		exprom_wp_set(ppd->dd, false);
+	else if (cdata == '1')
+		exprom_wp_set(ppd->dd, true);
+	else
+		return -EINVAL;
+
+	return 1;
+}
+
+static unsigned long exprom_in_use;
+
+static int exprom_wp_debugfs_open(struct inode *in, struct file *fp)
+{
+	if (test_and_set_bit(0, &exprom_in_use))
+		return -EBUSY;
+
+	return 0;
+}
+
+static int exprom_wp_debugfs_release(struct inode *in, struct file *fp)
+{
+	struct hfi1_pportdata *ppd = private2ppd(fp);
+
+	if (exprom_wp_disabled)
+		exprom_wp_set(ppd->dd, false);
+	clear_bit(0, &exprom_in_use);
+
+	return 0;
+}
+
 #define DEBUGFS_OPS(nm, readroutine, writeroutine)	\
 { \
 	.name = nm, \
@@ -1119,6 +1190,9 @@  static int qsfp2_debugfs_release(struct inode *in, struct file *fp)
 		     qsfp1_debugfs_open, qsfp1_debugfs_release),
 	DEBUGFS_XOPS("qsfp2", qsfp2_debugfs_read, qsfp2_debugfs_write,
 		     qsfp2_debugfs_open, qsfp2_debugfs_release),
+	DEBUGFS_XOPS("exprom_wp", exprom_wp_debugfs_read,
+		     exprom_wp_debugfs_write, exprom_wp_debugfs_open,
+		     exprom_wp_debugfs_release),
 	DEBUGFS_OPS("asic_flags", asic_flags_read, asic_flags_write),
 	DEBUGFS_OPS("dc8051_memory", dc8051_memory_read, NULL),
 	DEBUGFS_OPS("lcb", debugfs_lcb_read, debugfs_lcb_write),