On Tue, Jan 17, 2023 at 10:57:31AM -0600, Bob Pearson wrote: > >> - dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload); > >> - /* check vaddr is 8 bytes aligned. */ > >> - if (!dst || (uintptr_t)dst & 7) > >> - return RESPST_ERR_MISALIGNED_ATOMIC; > >> + if (res->replay) > >> + return RESPST_ACKNOWLEDGE; > >> > >> - /* Do atomic write after all prior operations have completed */ > >> - smp_store_release(dst, src); > >> + mr = qp->resp.mr; > >> + value = *(u64 *)payload_addr(pkt); > >> + iova = qp->resp.va + qp->resp.offset; > >> > >> - /* decrease resp.resid to zero */ > >> - qp->resp.resid -= sizeof(payload); > >> +#if defined CONFIG_64BIT > > > > Shouldn't need a #ifdef here > > This avoids a new special error (i.e. NOT_64_bit) and makes it clear we > won't call the code in mr. ? That doesn't seem right > I really don't understand why Fujitsu did it all this way instead of just > using a spinlock for 32 bit architectures as a fallback. But if I want to > keep to the spirit of their implementation this is fairly clear I think. IIRC the IBA definition is that this is supposed to be coherent with the host CPU, the spinlock version is for the non-atomic atomics which only has to be coherent with the "hca" So a spinlock will not provide coherency with userspace that may be touching this same atomic memory. Jason
