Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1497979

Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to struct crb_regs

From Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Newsgroups linux.kernel
Subject Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to struct crb_regs
Date 2016-10-09 18:50 +0200
Message-ID <sqpsu-6CC-27@gated-at.bofh.it> (permalink)
References <sqa0p-5wE-5@gated-at.bofh.it> <sqa0p-5wE-17@gated-at.bofh.it> <sqbIR-6zO-1@gated-at.bofh.it> <sqiKm-2pg-23@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Oct 09, 2016 at 12:38:18PM +0300, Jarkko Sakkinen wrote:
> On Sat, Oct 08, 2016 at 07:42:56PM -0600, Jason Gunthorpe wrote:
> > On Sun, Oct 09, 2016 at 03:15:09AM +0300, Jarkko Sakkinen wrote:
> > > +	ctrl = crb_map_res(dev, priv, &io_res, buf->control_address,
> > > +			   sizeof(struct crb_regs) -
> > > +			   offsetof(struct crb_regs, ctrl_req));
> > > +	if (IS_ERR(ctrl))
> > > +		return PTR_ERR(ctrl);
> > > +
> > > +	/* The control area always overrlaps IO memory mapped from the ACPI
> > > +	 * object with CRB start only devices. Thus, this is perfectly safe.
> > > +	 */
> > > +	priv->regs = (void *)((unsigned long)ctrl -
> > > +		offsetof(struct crb_regs, ctrl_req));
> > 
> > Hum. No, this makes bad assumptions about the structure of iomapping.
> > 
> > The map itself needs to be done with the adjustment:
> > 
> > 	ctrl = crb_map_res(dev, priv, &io_res, buf->control_address -
> > 		offsetof(struct crb_regs, ctrl_req),
> > 	   	sizeof(struct crb_regs));
> 
> That would be wrong address for the control area as it does not start
> from the beginning of CRB registers.

Of course, I just pointed out what the map call should look like

Something like this

 	priv->regs = crb_map_res(dev, priv, &io_res, buf->control_address -
 		offsetof(struct crb_regs, ctrl_req),
 	   	sizeof(struct crb_regs));
        ctrl = &priv->regs.ctrl_req;

> I think the crb_map_io and crb_map_res are too generic. Better way to do
> things would be to validate that assumptions for these two cases hold.

If the driver is going to be using a negative offset like this, then
it very much should validate the assumptions before doing it.

and not even map these regsiters if they are not supported by
hardware.

Jason

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH RFC 0/3] Locality support for the CRB driver Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 02:20 +0200
  [PATCH RFC 3/3] tpm_crb: request and relinquish locality 0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 02:20 +0200
    RE: [tpmdd-devel] [PATCH RFC 3/3] tpm_crb: request and relinquish         locality 0 "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-10-09 08:40 +0200
      Re: [tpmdd-devel] [PATCH RFC 3/3] tpm_crb: request and relinquish  locality 0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 11:30 +0200
        RE: [tpmdd-devel] [PATCH RFC 3/3] tpm_crb: request and relinquish  locality 0 "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-10-09 11:50 +0200
          Re: [tpmdd-devel] [PATCH RFC 3/3] tpm_crb: request and relinquish  locality 0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 12:50 +0200
  [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 02:20 +0200
    Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-09 04:10 +0200
      Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 11:40 +0200
        Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-09 18:50 +0200
          Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 20:10 +0200
          Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 20:40 +0200
            Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-10 01:10 +0200
              RE: [tpmdd-devel] [PATCH RFC 1/3] tpm_crb: expand struct  crb_control_area to struct crb_regs "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-10-10 02:30 +0200
                Re: [tpmdd-devel] [PATCH RFC 1/3] tpm_crb: expand struct  crb_control_area to struct crb_regs Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2016-10-10 05:30 +0200
                Re: [tpmdd-devel] [PATCH RFC 1/3] tpm_crb: expand struct  crb_control_area to struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-10 07:00 +0200
              Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-10 06:50 +0200
          Re: [PATCH RFC 1/3] tpm_crb: expand struct crb_control_area to  struct crb_regs Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-10-09 20:40 +0200

csiph-web