Hi Jens,
On 13/04/2023 08:14, Jens Wiklander wrote:
+static uint32_t handle_rxtx_map(uint32_t fid, register_t tx_addr,
+ register_t rx_addr, uint32_t page_count)
+{
+ uint32_t ret = FFA_RET_INVALID_PARAMETERS;
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.tee;
+ struct page_info *tx_pg;
+ struct page_info *rx_pg;
+ p2m_type_t t;
+ void *rx;
+ void *tx;
+
+ if ( !smccc_is_conv_64(fid) )
+ {
+ /*
+ * Calls using the 32-bit calling convention must ignore the upper
+ * 32 bits in the argument registers.
+ */
+ tx_addr &= UINT32_MAX;
+ rx_addr &= UINT32_MAX;
+ }
+
+ if ( page_count > FFA_MAX_RXTX_PAGE_COUNT ) {
Coding style:
if ( ... )
{
+ printk(XENLOG_ERR "ffa: RXTX_MAP: error: %u pages requested (limit
%u)\n",
+ page_count, FFA_MAX_RXTX_PAGE_COUNT);
+ return FFA_RET_NOT_SUPPORTED;
+ }
+
+ /* Already mapped */
+ if ( ctx->rx )
+ return FFA_RET_DENIED;
+
+ tx_pg = get_page_from_gfn(d, gfn_x(gaddr_to_gfn(tx_addr)), &t, P2M_ALLOC);
I might be missing something. Here you only get the reference on one
page. Per the value of FFA_MAX_RXTX_PAGE_COUNT, it looks like the buffer
can be up to 32 pages.
Can you clarify?
+ if ( !tx_pg )
+ return FFA_RET_INVALID_PARAMETERS;
+ /* Only normal RAM for now */
+ if ( !p2m_is_ram(t) )
p2m_is_ram() would allow RAM page marked read-only in stage-2. Is it
intended?
If not, then I think you want to use t != p2m_ram_rw.
+ goto err_put_tx_pg;
+
+ rx_pg = get_page_from_gfn(d, gfn_x(gaddr_to_gfn(rx_addr)), &t, P2M_ALLOC);
+ if ( !tx_pg )
+ goto err_put_tx_pg;
+ /* Only normal RAM for now */
+ if ( !p2m_is_ram(t) )
Same here.
+ goto err_put_rx_pg;
+
+ tx = __map_domain_page_global(tx_pg);
+ if ( !tx )
+ goto err_put_rx_pg;
+
+ rx = __map_domain_page_global(rx_pg);
+ if ( !rx )
+ goto err_unmap_tx;
+
+ ctx->rx = rx;
+ ctx->tx = tx;
+ ctx->rx_pg = rx_pg;
+ ctx->tx_pg = tx_pg;
+ ctx->page_count = page_count;
+ ctx->tx_is_free = true;
+ return FFA_RET_OK;
Cheers,
--
Julien Grall
