On Fri, May 06 2022, Eric Auger <eric.au...@redhat.com> wrote: > Hi Alex, > > On 4/28/22 22:14, Alex Williamson wrote: >> On Thu, 28 Apr 2022 15:49:45 +0200 >> Eric Auger <eric.au...@redhat.com> wrote:
>>> +static bool vfio_known_safe_misalignment(MemoryRegionSection *section) >>> +{ >>> + MemoryRegion *mr = section->mr; >>> + >>> + if (!TPM_IS_CRB(mr->owner)) { >>> + return false; >>> + } >> It looks like this test is going to need to be wrapped in #ifdef >> CONFIG_TPM: > > sorry for the delay. Your message fell though the cracks :-( > > if I put an '#ifdef CONFIG_TPM' I need to inverse the logic because by > default the function shall return false. > > solution #1 > > #ifdef CONFIG_TPM > if (TPM_IS_CRB(mr->owner)) { > > /* this is a known safe misaligned region, just trace for debug purpose */ > trace_vfio_known_safe_misalignment(memory_region_name(mr), > section->offset_within_address_space, > section->offset_within_region, > qemu_real_host_page_size()); > > return true; > } > > #endif > return false; > > This looks weird to me. > > + if (!object_dynamic_cast(mr->owner, TYPE_TPM_CRB)) { > + return false; > + } > > > solution #2 > replace !object_dynamic_cast(mr->owner, TYPE_TPM_CRB) by > !object_dynamic_cast(mr->owner, "tpm-crb") > and add a comment saying that we don't use TYPE_TPM_CRB on purpose > > solution #3 > Move #define TPM_IS_CRB(chr) and related defined out of > #ifdef CONFIG_TPM hoping it does not have other side effects > > Thoughts? > Eric solution #4 #ifndef CONFIG_TPM /* needed for an alignment check in non-tpm code */ static inline Object *TPM_IS_CRB(Object *obj) { return NULL; } #endif I think it would be good if we could hide the configuration details in the header.