| Message ID | f267f3ce-9baa-5e2f-1f0a-c08e59a53a7a@bell.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | parisc: Use implicit space register selection for loading the coherence index of I/O pdirs | expand |
On 28.05.19 02:15, John David Anglin wrote: > We only support I/O to kernel space. Using %sr1 to load the coherence index > may be racy unless interrupts are disabled. This patch changes the code used > to load the coherence index to use implicit space register selection. This saves > one instruction and eliminates the race. Fun part is, that I had prepared exactly the same patch two days ago too. In addition I added this: + /* We currently only support kernel addresses, and sr0 is always 0. */ + /* BUG_ON(mfsp(0) != sid); */ and explicitely mentioned "%sr0" to make it clear: asm volatile ("lci %%r0(%sr0,%1), %0" : "=r" (ci) : "r" (vba)); Anyway, your patch is good. Will apply. Helge > > Tested on rp3440, c8000 and c3750. > > Signed-off-by: John David Anglin <dave.anglin@bell.net> > --- > > diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c > index acba1f56af3e..d7649a70a0c4 100644 > --- a/drivers/parisc/ccio-dma.c > +++ b/drivers/parisc/ccio-dma.c > @@ -565,8 +565,6 @@ ccio_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, > /* We currently only support kernel addresses */ > BUG_ON(sid != KERNEL_SPACE); > > - mtsp(sid,1); > - > /* > ** WORD 1 - low order word > ** "hints" parm includes the VALID bit! > @@ -597,7 +595,7 @@ ccio_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, > ** Grab virtual index [0:11] > ** Deposit virt_idx bits into I/O PDIR word > */ > - asm volatile ("lci %%r0(%%sr1, %1), %0" : "=r" (ci) : "r" (vba)); > + asm volatile ("lci %%r0(%1), %0" : "=r" (ci) : "r" (vba)); > asm volatile ("extru %1,19,12,%0" : "+r" (ci) : "r" (ci)); > asm volatile ("depw %1,15,12,%0" : "+r" (pa) : "r" (ci)); > > diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c > index 0a9c762a70fa..5468490d2298 100644 > --- a/drivers/parisc/sba_iommu.c > +++ b/drivers/parisc/sba_iommu.c > @@ -575,8 +575,7 @@ sba_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, > pa = virt_to_phys(vba); > pa &= IOVP_MASK; > > - mtsp(sid,1); > - asm("lci 0(%%sr1, %1), %0" : "=r" (ci) : "r" (vba)); > + asm("lci 0(%1), %0" : "=r" (ci) : "r" (vba)); > pa |= (ci >> PAGE_SHIFT) & 0xff; /* move CI (8 bits) into lowest byte */ > > pa |= SBA_PDIR_VALID_BIT; /* set "valid" bit */ >
On 2019-05-28 1:01 a.m., Helge Deller wrote: > Fun part is, that I had prepared exactly the same patch two days ago too. > In addition I added this: > + /* We currently only support kernel addresses, and sr0 is always 0. */ > + /* BUG_ON(mfsp(0) != sid); */ > > and explicitely mentioned "%sr0" to make it clear: > asm volatile ("lci %%r0(%sr0,%1), %0" : "=r" (ci) : "r" (vba)); Personally, I prefer not to mention %sr0 in instructions that use 2-bit space id. The special case where s=0 causes the selected space register to be determined from the %1 operand. The selected space register will be %sr4, %sr5, %sr6 or %sr7. These are all 0 when running kernel code. Dave
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c index acba1f56af3e..d7649a70a0c4 100644 --- a/drivers/parisc/ccio-dma.c +++ b/drivers/parisc/ccio-dma.c @@ -565,8 +565,6 @@ ccio_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, /* We currently only support kernel addresses */ BUG_ON(sid != KERNEL_SPACE); - mtsp(sid,1); - /* ** WORD 1 - low order word ** "hints" parm includes the VALID bit! @@ -597,7 +595,7 @@ ccio_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, ** Grab virtual index [0:11] ** Deposit virt_idx bits into I/O PDIR word */ - asm volatile ("lci %%r0(%%sr1, %1), %0" : "=r" (ci) : "r" (vba)); + asm volatile ("lci %%r0(%1), %0" : "=r" (ci) : "r" (vba)); asm volatile ("extru %1,19,12,%0" : "+r" (ci) : "r" (ci)); asm volatile ("depw %1,15,12,%0" : "+r" (pa) : "r" (ci)); diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c index 0a9c762a70fa..5468490d2298 100644 --- a/drivers/parisc/sba_iommu.c +++ b/drivers/parisc/sba_iommu.c @@ -575,8 +575,7 @@ sba_io_pdir_entry(u64 *pdir_ptr, space_t sid, unsigned long vba, pa = virt_to_phys(vba); pa &= IOVP_MASK; - mtsp(sid,1); - asm("lci 0(%%sr1, %1), %0" : "=r" (ci) : "r" (vba)); + asm("lci 0(%1), %0" : "=r" (ci) : "r" (vba)); pa |= (ci >> PAGE_SHIFT) & 0xff; /* move CI (8 bits) into lowest byte */ pa |= SBA_PDIR_VALID_BIT; /* set "valid" bit */
We only support I/O to kernel space. Using %sr1 to load the coherence index may be racy unless interrupts are disabled. This patch changes the code used to load the coherence index to use implicit space register selection. This saves one instruction and eliminates the race. Tested on rp3440, c8000 and c3750. Signed-off-by: John David Anglin <dave.anglin@bell.net> ---