On 1/17/23 10:59, Jason Gunthorpe wrote: > 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 that was the -3 of the -1, -2, -3 that we just fixed. there are three error paths out of this state and we need a way to get to them. The #ifdef provides that third path. > >> 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. Thanks. That makes sense. Bob > > Jason
