Hi On Thu, Mar 3, 2022 at 6:41 PM Eric Auger <eau...@redhat.com> wrote:
> Hi Stefan, > > On 2/8/22 6:58 PM, Eric Auger wrote: > > Hi Stefan, > > > > On 2/8/22 6:16 PM, Stefan Berger wrote: > >> > >> On 2/8/22 08:38, Eric Auger wrote: > >>> Representing the CRB cmd/response buffer as a standard > >>> RAM region causes some trouble when the device is used > >>> with VFIO. Indeed VFIO attempts to DMA_MAP this region > >>> as usual RAM but this latter does not have a valid page > >>> size alignment causing such an error report: > >>> "vfio_listener_region_add received unaligned region". > >>> To allow VFIO to detect that failing dma mapping > >>> this region is not an issue, let's use a ram_device > >>> memory region type instead. > >>> > >>> Signed-off-by: Eric Auger <eric.au...@redhat.com> > >>> Tested-by: Stefan Berger <stef...@linux.ibm.com> > >>> Acked-by: Stefan Berger <stef...@linux.ibm.com> > >>> [PMD: Keep tpm_crb.c in meson's softmmu_ss] > >>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > >> > >> > >> v4 doesn't build for me: > >> > >> ../hw/tpm/tpm_crb.c: In function ?tpm_crb_realize?: > >> ../hw/tpm/tpm_crb.c:297:33: error: implicit declaration of function > >> ?HOST_PAGE_ALIGN? [-Werror=implicit-function-declaration] > >> 297 | HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE)); > >> | ^~~~~~~~~~~~~~~ > >> ../hw/tpm/tpm_crb.c:297:33: error: nested extern declaration of > >> ?HOST_PAGE_ALIGN? [-Werror=nested-externs] > >> cc1: all warnings being treated as errors > > > > Do you have > > b269a70810a exec/cpu: Make host pages variables / macros 'target > > agnostic' in your tree? > I may have missed your reply. Did you have that dependency? Were you > able to compile eventually? > > Besides, do you have any opinion overall about the relevance of > transforming the CRB ctrl cmd region into a RAM device wrt the TPM spec? > > Again spec says: > > " > Including the control structure, the three memory areas comprise the > entirety of the CRB. There are no constraints on how those three memory > areas are provided. They can all be in system RAM, or all be in device > memory, or any combination. > " > ( > https://trustedcomputinggroup.org/wp-content/uploads/Mobile-Command-Response-Buffer-Interface-v2-r12-Specification_FINAL2.pdf > ) > > What was the rationale behind using RAM device for the PPI region? > Is this the rationale you are looking for? https://gitlab.com/qemu-project/qemu/-/commit/3b97c01e9ccdfbd517a0fd631838d6252dbfa692 Note: bios_linker cannot be used to allocate the PPI memory region, since the reserved memory should stay stable across reboots, and might be needed before the ACPI tables are installed. > > There are some spurious warnings when using CRB with VFIO and that would > be nice to remove them one way or the other. > > Thanks > > Eric > > > > Thanks > > > > Eric > >> > >> > >> > > > > > -- Marc-André Lureau
