Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 8 Jul 2022 16:04:05 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Ira Weiny <ira.weiny@intel.com>
Cc:     Bjorn Helgaas <helgaas@kernel.org>,
        Dan Williams <dan.j.williams@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org,
        linux-pci@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH 2/3] pci/doe: Use devm_xa_init()
Message-ID: <YshHZXK/dq3apNDu@casper.infradead.org>
References: <20220705232159.2218958-3-ira.weiny@intel.com>
 <20220707160646.GA306751@bhelgaas>
 <YshC+Jaua01dPQak@iweiny-desk3>
 <YshED+nm7LdcmL75@casper.infradead.org>
 <YshFxnBZGUPN5LoC@iweiny-desk3>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YshFxnBZGUPN5LoC@iweiny-desk3>
Precedence: bulk

On Fri, Jul 08, 2022 at 07:57:10AM -0700, Ira Weiny wrote:
> > > I'll update this to be more clear in a V1 if it goes that far.  But to clarify
> > > here; the protocol information is a u16 vendor id and u8 protocol number.  So
> > > we are able to store that in the unsigned long value that would normally be a
> > > pointer to something in the XArray.
> > 
> > Er.  Signed long.
> 
> Sorry I misspoke, xa_mk_value() takes an unsigned long.

It does, *but* ...

static inline void *xa_mk_value(unsigned long v)
{
        WARN_ON((long)v < 0);
        return (void *)((v << 1) | 1);
}

... you can't pass an integer that has the top bit set to it.

> Can't I use xa_mk_value() to store data directly in the entry "pointer"?

Yes, that's the purpose of xa_mk_value().  From what you said, it sounded
like you were just storing the integer directly, which won't work.

> +static void *pci_doe_xa_prot_entry(u16 vid, u8 prot)
> +{
> +	return xa_mk_value(((unsigned long)vid << 16) | prot);
> +}
> 
> Both Dan and I thought this was acceptable in XArray?

You haven't tested that on 32-bit, have you?  Shift vid by 8 instead of
16, and it'll be fine.

(Oh, and you don't need to cast vid; the standard C integer promotions
will promote vid to int before shifting, and you won't lose any bits)