From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 236A0C4167B for ; Sat, 2 Dec 2023 01:23:02 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 86AF810E13B; Sat, 2 Dec 2023 01:23:01 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3497110E13B for ; Sat, 2 Dec 2023 01:22:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701480178; x=1733016178; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=s2TC3jjY6snHe16mffudL7GVgK6eYADu4+EXyC6zKbQ=; b=gGDSliLDaCHXtGg8f4AmjJMpDYD9kjShGoHFMP/tnf6Zb69MLk2pbuTx liEKGyzqi6x+LGtoEEVhtS76yAT00hzV85Ob9rrOU6eCuD8b2BCjmIqLf DPmzddrzygqTBqEkbOPfvTBScp2JQqNzxMV043oJ0UW+zf5GfprY9H7Vn avSkeTCkkhDMrtlIXCdr0xmSH8IJNmmHd+x2WpL+AS0Y09RgxQhk1bL7L zjyHAHWRim1FTfCWSvx9XNfGdssTP99D1ZCNQgNDsp4hpsZ8onMuXLX2f azjKqngBAhQPi/AovLlfBSrxSwCLMvu0VRwW0G74zrM28iydQR8AiF7BL Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10911"; a="15116942" X-IronPort-AV: E=Sophos;i="6.04,243,1695711600"; d="scan'208";a="15116942" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2023 17:22:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10911"; a="1017199572" X-IronPort-AV: E=Sophos;i="6.04,243,1695711600"; d="scan'208";a="1017199572" Received: from adixit-mobl.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.209.27.211]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2023 17:22:52 -0800 Date: Fri, 01 Dec 2023 17:10:14 -0800 Message-ID: <87o7f9if49.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Francois Dugast In-Reply-To: <20231130183955.7-16-francois.dugast@intel.com> References: <20231130183955.7-1-francois.dugast@intel.com> <20231130183955.7-16-francois.dugast@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-xe] [PATCH v3 15/16] drm/xe: Remove unused extension definition X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-xe@lists.freedesktop.org, Rodrigo Vivi Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 30 Nov 2023 10:39:54 -0800, Francois Dugast wrote: > > From: Rodrigo Vivi > > The vm_create ioctl function doesn't accept any extension. > Remove this left over. > A backward compatible change. > > Cc: Francois Dugast > Signed-off-by: Rodrigo Vivi > Reviewed-by: Matthew Brost > --- > include/uapi/drm/xe_drm.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > index 84a477009e85..ffd9fc1172e8 100644 > --- a/include/uapi/drm/xe_drm.h > +++ b/include/uapi/drm/xe_drm.h > @@ -642,7 +642,6 @@ struct drm_xe_ext_set_property { > }; > > struct drm_xe_vm_create { > -#define DRM_XE_VM_EXTENSION_SET_PROPERTY 0 > /** @extensions: Pointer to the first extension struct, if any */ > __u64 extensions; I have another general comment on the use of these extensions (prompted by John H's bringing up KLV's (key-length-value blobs) in an internal chat). Here goes. Currently we have structs which are basically: struct xyz { /** @extensions: Pointer to the first extension struct, if any */ __u64 extensions; /* Remaining struct fields */ }; A slightly different way of doing this would be: struct root { /** @extensions: Pointer to the first extension struct */ __u64 extensions; }; struct xyz { /** @base: base user extension */ struct xe_user_extension base; /* Remaining struct fields */ }; So here the idea is the the first level struct ('struct root') does not contain any real struct fields at all. It *always* points to a second level struct ('struct xyz') which contain the real data to passed in. Maybe what we have in Xe today is ok, but the second approach is closer to KLV John brought up. If 'struct xyz' changes, we can just change 'extensions' in 'struct root' to point to say a new 'struct xyz_v2'. What we have now in Xe is also ok, but we need a protocol for how to interpret 'extensions', e.g. that if 'extensions' is 0, we will use the first level struct, but if 'extensions' is not 0, we will follow the chain and use the second level struct. As long as this protocol is clearly defined we might be ok with what we have. But is the protocol defined today? Is it the same behavior across all uapi's? That's why the second approach seems a bit cleaner and unambiguous to me. Thanks. -- Ashutosh