From patchwork Fri May 12 03:20:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Li, Weinan Z" X-Patchwork-Id: 9723501 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7A5D760348 for ; Fri, 12 May 2017 03:20:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 71F12287CF for ; Fri, 12 May 2017 03:20:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 70A6A287E0; Fri, 12 May 2017 03:20:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 17248287D8 for ; Fri, 12 May 2017 03:20:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 24F1B6E633; Fri, 12 May 2017 03:20:17 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 126B96E07A; Fri, 12 May 2017 03:20:15 +0000 (UTC) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 May 2017 20:20:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,327,1491289200"; d="scan'208";a="86643506" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 11 May 2017 20:20:14 -0700 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 11 May 2017 20:20:13 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 11 May 2017 20:20:13 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.193]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.224]) with mapi id 14.03.0319.002; Fri, 12 May 2017 11:20:10 +0800 From: "Li, Weinan Z" To: Joonas Lahtinen , "intel-gfx@lists.freedesktop.org" , "intel-gvt-dev@lists.freedesktop.org" Thread-Topic: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Thread-Index: AQHSyTpV2KN7NA1SQk+gguwEHle5eaHs272AgAGS0uCAACS+AIABdGlg Date: Fri, 12 May 2017 03:20:10 +0000 Message-ID: <9BD218709B5F2A4F96F08B4A3B98A89768551312@SHSMSX101.ccr.corp.intel.com> References: <1494385161-25662-1-git-send-email-weinan.z.li@intel.com> <1494412963.6362.18.camel@linux.intel.com> <9BD218709B5F2A4F96F08B4A3B98A89768550F4E@SHSMSX101.ccr.corp.intel.com> <1494507358.4542.28.camel@linux.intel.com> In-Reply-To: <1494507358.4542.28.camel@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Intel-gfx] [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Thanks. Best Regards. Weinan, LI > -----Original Message----- > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com] > Sent: Thursday, May 11, 2017 8:56 PM > To: Li, Weinan Z ; intel-gfx@lists.freedesktop.org; intel- > gvt-dev@lists.freedesktop.org > Cc: Chris Wilson > Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size > under gvt environment > > On to, 2017-05-11 at 06:51 +0000, Li, Weinan Z wrote: > > > > > > -----Original Message----- > > > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com] > > > Sent: Wednesday, May 10, 2017 6:43 PM > > > To: Li, Weinan Z ; > > > intel-gfx@lists.freedesktop.org; intel- > > > gvt-dev@lists.freedesktop.org > > > Cc: Chris Wilson > > > Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable > > > aperture size under gvt environment > > > > > > On ke, 2017-05-10 at 10:59 +0800, Weinan Li wrote: > > > > > > > > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from > > > userspace. > > > > > > > > In gvt environment, each vm only use the ballooned part of > > > > aperture, so we should return the correct available aperture size > > > > exclude the reserved part by balloon. > > > > > > > > v2: add 'reserved' in struct i915_address_space to record the > > > > reserved size in ggtt. > > > > > > > > v3: remain aper_size as total, adjust aper_available_size exclude > > > > reserved and pinned. UMD driver need to adjust the max allocation > > > > size according to the available aperture size but not total size. > > > > KMD return the correct usable aperture size any time. > > > > > > > > v4: add onion teardown to balloon and deballoon to make sure the > > > > reserved stays correct. Code style refine. > > > > > > There's no onion teardown seen yet, please read: > > > > > > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#cent > > > ral > > > ized-exiting-of-functions > > > > > > Please incorporate the addition to vgt_balloon_space function to > > > reduce code duplication. > > > > > > Once the proper teardown is implemented, intel_vgt_deballoon > > > function should unconditionally remove the drm_mm nodes as there's > > > no condition when only one of them would be allocated. And > > > intel_vgt_balloon definitely should not call intel_vgt_deballoon in error > path as per the coding style above. > > > > Thanks Joonas. A little change is brought in if move the fail checking > > into balloon_space() There are 4 balloon spaces, current policy is if > > any one fail clean up all the success ones, with this change it won't clean up > the success ones. It should not impact the driver's behavior. > > Please re-read the "Centralized exiting of function", in this case you'd have > three labels for onion teardown if any of the space reservations fails, you jump > to the right one. Please take a look in the i915 to similar initialization functions > for examples. > > > @@ -127,9 +130,14 @@ static int vgt_balloon_space(struct i915_ggtt > > *ggtt, > > > >         DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n", > >                  start, end, size / 1024); > > -       return i915_gem_gtt_reserve(&ggtt->base, node, > > +       ret = i915_gem_gtt_reserve(&ggtt->base, node, > >                                     size, start, > > I915_COLOR_UNEVICTABLE, > >                                     0); > > +       if (!ret) > > +               ggtt->base.reserved += size; > > +       else > > +               memset(node, 0, sizeof(*node)); > > This should not be needed. Either all of the nodes have been successfully > initialize or not. The only partial states are in the intel_vgt_balloon function, > which should use different labels to back off from different stages of > initialization. Thanks Joonas' guidance. I add 4 labels in intel_vgt_balloon for cleaning up ballooned space, the reserved size will increase when one balloon space reserve success, and will clean up if anyone reserve fail step by step. > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c index 4ab8a97..e21cfff 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.c +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -109,8 +109,8 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv) DRM_DEBUG("VGT deballoon.\n"); for (i = 0; i < 4; i++) { - if (bl_info.space[i].allocated) - drm_mm_remove_node(&bl_info.space[i]); + dev_priv->ggtt.base.reserved -= bl_info.space[i].size; + drm_mm_remove_node(&bl_info.space[i]); } memset(&bl_info, 0, sizeof(bl_info)); @@ -120,6 +120,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt, struct drm_mm_node *node, unsigned long start, unsigned long end) { + int ret; unsigned long size = end - start; if (start >= end) @@ -127,9 +128,12 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt, DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n", start, end, size / 1024); - return i915_gem_gtt_reserve(&ggtt->base, node, + ret = i915_gem_gtt_reserve(&ggtt->base, node, size, start, I915_COLOR_UNEVICTABLE, 0); + if (!ret) + ggtt->base.reserved += size; + return ret; } /** @@ -215,14 +219,14 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv) ggtt->mappable_end, unmappable_base); if (ret) - goto err; + goto out_err; } if (unmappable_end < ggtt_end) { ret = vgt_balloon_space(ggtt, &bl_info.space[3], unmappable_end, ggtt_end); if (ret) - goto err; + goto deballoon_upon_mappable; } /* Mappable graphic memory ballooning */ @@ -231,7 +235,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv) 0, mappable_base); if (ret) - goto err; + goto deballoon_upon_unmappable; } if (mappable_end < ggtt->mappable_end) { @@ -239,14 +243,23 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv) mappable_end, ggtt->mappable_end); if (ret) - goto err; + goto deballoon_below_mappable; } DRM_INFO("VGT balloon successfully\n"); return 0; -err: +deballoon_below_mappable: + ggtt->base.reserved -= bl_info.space[0].size; + drm_mm_remove_node(&bl_info.space[0]); +deballoon_upon_unmappable: + ggtt->base.reserved -= bl_info.space[3].size; + drm_mm_remove_node(&bl_info.space[3]); +deballoon_upon_mappable: + ggtt->base.reserved -= bl_info.space[2].size; + drm_mm_remove_node(&bl_info.space[2]); +out_err: DRM_ERROR("VGT balloon fail\n"); - intel_vgt_deballoon(dev_priv); + memset(&bl_info, 0, sizeof(bl_info)); return ret; }