From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 2/2] x86/vMSI-X: add valid bits for read acceleration Date: Mon, 23 Mar 2015 15:11:46 -0400 Message-ID: <20150323191146.GA20509@l.oracle.com> References: <550C5065020000780006C27E@mail.emea.novell.com> <550C589D020000780006C2B2@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Ya7lT-0007Vf-Vc for xen-devel@lists.xenproject.org; Mon, 23 Mar 2015 19:11:56 +0000 Content-Disposition: inline In-Reply-To: <550C589D020000780006C2B2@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Keir Fraser , Andrew Cooper List-Id: xen-devel@lists.xenproject.org On Fri, Mar 20, 2015 at 04:27:57PM +0000, Jan Beulich wrote: > Again because Xen doesn't get to see all guest writes, it shouldn't > serve reads from its cache before having seen a write to the respective > address. > > Signed-off-by: Jan Beulich Reviewed-by: Konrad Rzeszutek Wilk > > --- a/xen/arch/x86/hvm/vmsi.c > +++ b/xen/arch/x86/hvm/vmsi.c > @@ -153,12 +153,15 @@ struct msixtbl_entry > /* TODO: resolve the potential race by destruction of pdev */ > struct pci_dev *pdev; > unsigned long gtable; /* gpa of msix table */ > - unsigned long table_flags[BITS_TO_LONGS(MAX_MSIX_TABLE_ENTRIES)]; > + DECLARE_BITMAP(table_flags, MAX_MSIX_TABLE_ENTRIES); That seems unrelated to this patch? Perhaps mention the cleanup part in the commit. > #define MAX_MSIX_ACC_ENTRIES 3 > unsigned int table_len; > struct { > uint32_t msi_ad[3]; /* Shadow of address low, high and data */ > } gentries[MAX_MSIX_ACC_ENTRIES]; > + DECLARE_BITMAP(acc_valid, 3 * MAX_MSIX_ACC_ENTRIES); > +#define acc_bit(what, ent, slot, idx) \ > + what##_bit((slot) * 3 + (idx), (ent)->acc_valid) > struct rcu_head rcu; > }; > > @@ -233,9 +236,10 @@ static int msixtbl_read( > if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET ) > { > nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE; > - if ( nr_entry >= MAX_MSIX_ACC_ENTRIES ) > - goto out; > index = offset / sizeof(uint32_t); > + if ( nr_entry >= MAX_MSIX_ACC_ENTRIES || > + !acc_bit(test, entry, nr_entry, index) ) > + goto out; > *pval = entry->gentries[nr_entry].msi_ad[index]; > } > else > @@ -281,6 +285,7 @@ static int msixtbl_write(struct vcpu *v, > { > index = offset / sizeof(uint32_t); > entry->gentries[nr_entry].msi_ad[index] = val; > + acc_bit(set, entry, nr_entry, index); > } > set_bit(nr_entry, &entry->table_flags); > goto out; > > > > x86/vMSI-X: add valid bits for read acceleration > > Again because Xen doesn't get to see all guest writes, it shouldn't > serve reads from its cache before having seen a write to the respective > address. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/hvm/vmsi.c > +++ b/xen/arch/x86/hvm/vmsi.c > @@ -153,12 +153,15 @@ struct msixtbl_entry > /* TODO: resolve the potential race by destruction of pdev */ > struct pci_dev *pdev; > unsigned long gtable; /* gpa of msix table */ > - unsigned long table_flags[BITS_TO_LONGS(MAX_MSIX_TABLE_ENTRIES)]; > + DECLARE_BITMAP(table_flags, MAX_MSIX_TABLE_ENTRIES); > #define MAX_MSIX_ACC_ENTRIES 3 > unsigned int table_len; > struct { > uint32_t msi_ad[3]; /* Shadow of address low, high and data */ > } gentries[MAX_MSIX_ACC_ENTRIES]; > + DECLARE_BITMAP(acc_valid, 3 * MAX_MSIX_ACC_ENTRIES); > +#define acc_bit(what, ent, slot, idx) \ > + what##_bit((slot) * 3 + (idx), (ent)->acc_valid) > struct rcu_head rcu; > }; > > @@ -233,9 +236,10 @@ static int msixtbl_read( > if ( offset != PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET ) > { > nr_entry = (address - entry->gtable) / PCI_MSIX_ENTRY_SIZE; > - if ( nr_entry >= MAX_MSIX_ACC_ENTRIES ) > - goto out; > index = offset / sizeof(uint32_t); > + if ( nr_entry >= MAX_MSIX_ACC_ENTRIES || > + !acc_bit(test, entry, nr_entry, index) ) > + goto out; > *pval = entry->gentries[nr_entry].msi_ad[index]; > } > else > @@ -281,6 +285,7 @@ static int msixtbl_write(struct vcpu *v, > { > index = offset / sizeof(uint32_t); > entry->gentries[nr_entry].msi_ad[index] = val; > + acc_bit(set, entry, nr_entry, index); > } > set_bit(nr_entry, &entry->table_flags); > goto out; > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel