From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacob Pan Subject: Re: bind pasid table API Date: Wed, 27 Sep 2017 10:51:55 -0700 Message-ID: <20170927105155.0dddbc2d@jacob-builder> References: <20170918204516.2f6beffb@jacob-builder> <6ecc1afc-6302-cd22-6944-ef4c6ac09587@arm.com> <20170927134041.GN8398@8bytes.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170927134041.GN8398-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Joerg Roedel Cc: "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, David Woodhouse List-Id: iommu@lists.linux-foundation.org On Wed, 27 Sep 2017 15:40:41 +0200 Joerg Roedel wrote: > Hi, > > On Wed, Sep 20, 2017 at 01:09:47PM +0100, Jean-Philippe Brucker wrote: > > For binding page tables instead of PASID tables (e.g. > > virtio-iommu), the generic data would be: > > > > struct pgtable_info { > > __u32 pasid; > > __u64 ptr; > > __u32 model; > > __u8 model_data[]; > > }; > > I had a look again and at the AMD side there is no way to build a > shadow pasid-table when more than 9-bit pasids are used, because all > pointers in that multi-level table are GPA and thus translated. > > So how about allowing the virtio-iommu to build pasid-tables in > different formats? We can easily collect the existing code for that in > the Intel and AMD drivers into a library that could be used by the > virtio-iommu driver too. Then the virtio-iommu can create the correct > table for the host-iommu and we don't have to rely on shadowing. > > For binding whole pasid tables, reading through the thread, it looks > like we can't get away with generic attributes that would fit > everyone. For AMD and Intel it would suffice to have a base-ptr and > the number of pasids-bits the table can map. > > The Intel driver can then calculate the size of the table and the AMD > driver can compute the GLX value; > > In case we need the model-specific info, I'd like to have them > explicitly stated in the struct: > > enum pasid_table_model { > PASID_TABLE_INTEL, > PASID_TABLE_ARM, > PASID_TABLE_AMD, > /* ... */ > }; > I guess one vendor could have multiple pasid table format. so perhaps the name could reflect the format as well? > struct pasid_table_config { > __u64 base_ptr; > __u8 pasid_bits; > union { > struct { > /* Intel specific fields */ > } intel; > > struct { > /* ARM specific fields */ > } arm; > > > struct { > /* AMD specific fields */ > } amd; > > /* ... */ > }; > }; > > How does that look? > It should work for us for now but I am not sure how stable the vendor specific fields will be, this is UAPI. BTW, do you also intend to include the pasid_table_model # in pasid_table_config? > Regards, > > Joerg > [Jacob Pan]