Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f903e2b9-f85b-a4c8-4706-f463919723a3@nvidia.com>
Date:   Wed, 27 Jul 2022 11:37:02 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v5 1/5] vfio: Add the device features for the low power
 entry and exit
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     Alex Williamson <alex.williamson@redhat.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Yishai Hadas <yishaih@nvidia.com>,
        Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
        Kevin Tian <kevin.tian@intel.com>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Max Gurtovoy <mgurtovoy@nvidia.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-pm@vger.kernel.org, linux-pci@vger.kernel.org
References: <20220719121523.21396-1-abhsahu@nvidia.com>
 <20220719121523.21396-2-abhsahu@nvidia.com>
 <20220721163445.49d15daf.alex.williamson@redhat.com>
 <aaef2e78-1ed2-fe8b-d167-8ea2dcbe45b6@nvidia.com>
 <20220725160928.43a17560.alex.williamson@redhat.com>
 <bd7bca18-ae07-c04a-23d3-bf71245da0cc@nvidia.com>
 <20220726172356.GH4438@nvidia.com>
From:   Abhishek Sahu <abhsahu@nvidia.com>
In-Reply-To: <20220726172356.GH4438@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TjRQODdxalVwWkJNL2tkVTJXZ3hFV0p3RHV3NVdtM2RWZHo2cS8yZjk2Njda?=
 =?utf-8?B?VVY1TjNCR2wxdk5XYytnWUxyUGQ0aUljVVRDZG5nZEdNc05JWlNrc2xHb2R5?=
 =?utf-8?B?RHgyaFozUUdYSXNYUy8vcUk2R2dGN05kL3ViQnB2K2ZITU5oTWkycHhiWkI1?=
 =?utf-8?B?NER0cTEydGZ1b05xRVdDNWtSS1VVQmxjS2o4ZXJzZmpNTmxoOEgxOG82NVFQ?=
 =?utf-8?B?VUl6cS9KMXRqWlM0UlJ3T2RvTm15Yktad2lmdjFSeHl1MkpPUlIvdlZPYkxC?=
 =?utf-8?B?RjViKzE1bnJKZGRrRlFjQVRGb2dvdUpDdG4remZQR21KT1JIZHlKVVhBTU9y?=
 =?utf-8?B?SkRpUkZ5cVRGMXp6NW9mRERaWEV6REJwL2NFaFloeGhMUnhpVWpScWNhNHJ3?=
 =?utf-8?B?bjlGSzlwWkhqcGEvVUZYaGZlc0tZTTVPWUVvR3VJZHlYRy9hMkNEY2xaaE1F?=
 =?utf-8?B?SURCU3BCVXRnR28wV2ptQ0hyNGNIR0xWekpBZDF1WWlTODA4UTJJYjFRQ0pY?=
 =?utf-8?B?NHUxUUJSYU12N0x0MmlwTTMwNjdtd2U4UjRaYjArV0NIUlZHOU5iVEtUaHpo?=
 =?utf-8?B?NVROd0ZTRTFxU29jZTI1cW1mMStCeUozaWFoWmxtNk9TVlgvSFFFTnJMWUlv?=
 =?utf-8?B?THpTUFJYQi9CT0hpb2o3WG5iRDV0ZStZbCtaSFJzV05nZFZkNExRaHdlbTFR?=
 =?utf-8?B?eTZtNkhBSnRCVys0R3RIMXphVEhpbTlTelJUeXR3S1Zid3I3UnQwU2J5bnh2?=
 =?utf-8?B?NDdjU3Qvd2V0OEFIdG4zNkdHUDM3QjE0MmZ6VG9oSkUrNkRMYmJxQ0Z1RWtS?=
 =?utf-8?B?QzlwT0hkbFFkeWRmNE41d2xhTXdUN09KRmxuQitBS1JTcDFXOFFQWDloMXFo?=
 =?utf-8?B?YTFWZXlYZmhkZm5LK0FUVk9YNUVOWnBjTjN6NUduWkdnWWJvT2psS1dmZGpB?=
 =?utf-8?B?dFphbG5aSnlFOGczY1pOVkoybFlBVXBaT3dNbmdGQzgxV3VlUE5xYWFPclpC?=
 =?utf-8?B?UzBXZDMxOWNHcXk0UlI2bEFsOW0vc1IyTmNmSjNFK0tNOHgzSWJDb29BZXVF?=
 =?utf-8?B?L3FxaW1Mdmc0TDZzUVJtZjl1a01sRVhJVEkxMElOdDBLWDkwZWZuQk95SGRq?=
 =?utf-8?B?RXgrVUtzWlhoT2tkYk5aZVFhcFA1L1RDWmk5ellKMU5QclRFaHRxbk1kb0U2?=
 =?utf-8?B?ZHVDanA2RC9WeXpaU0Jzc3h6TGYxeFQyWGQxaFBtUDV2VHpLOWlZK1BGS3Vz?=
 =?utf-8?B?cVNGcGpXM2RSb0U3TTkwd3JEVVJtZDFUUzFlWkxVYTBubkc4c1VMekF5OHVm?=
 =?utf-8?B?OG5BMW1lQVR6SU1ERzVVN1VtbmNGMFRaTXFkVDlYdWowYUNxbXROTXhDdHhr?=
 =?utf-8?B?SEwraEg5VUN1QUorMldKdU1iTTdHYXVuU0FFLzRwem1LMGt2NHN1NHZSeWpD?=
 =?utf-8?B?UWU1WlZWQXRPVXdEbTFGMVNMcVlzUFRzaFo4RmNWTnhObTZJYXNES0FKQytU?=
 =?utf-8?B?bm9KMTZlOGJscGRjQzI3SDBuOEpWeGtEWHN3L0FSYnNXZTJRQkpnN2EvU3Vk?=
 =?utf-8?B?OWozc0c1Vms5d0F6MUZtMjkwV2V6N2x6UFNYcFVndXlKMkFGMG0zTnFUbGE4?=
 =?utf-8?B?cGZBZURCRVN4a250Y2pqREtRQjFFUnFFcFZwV2Q4bDB4MG5lT0xNNGMrUFZa?=
 =?utf-8?B?Nk9nTENYZ0dxZ29NRFVvN2QvQzJVK21ZK3FVR3FSdVRDdEZQc0c0SnZvNkp0?=
 =?utf-8?B?cWVjc1dIcWZQNm1nQzZzenNZRUR3aDl4OE1XRFh5N29CUERnL1ZnS2JySFI5?=
 =?utf-8?B?aGF5QThwaVNuWGx2ZnRTeHkwZTFPa0RDd0Mva3VQZld6M3c4Sm1IdlIxNElS?=
 =?utf-8?B?WHl5TWgzUms1aHp0TFV5YTVkWGxqYTJXWFB0TUs2RmVMNVB4ZlNqMlJjUTZu?=
 =?utf-8?B?blZtYngxL1FoS2pqcnlZaFZjcU5JSjQ0NzZFb29rVWtFMTkwVHRqNGFHUTF0?=
 =?utf-8?B?a0E2RkxOSktFWXVocGdQdWhaZnBEVnpTNVpzMmJzMUd2eC8xMi9WRE0zZlJ4?=
 =?utf-8?B?U0JqM0dQUWZ3bEc3T2NhS1FBeitmaDNLTDNXQmt3K2hMc1VJNnEyQUI5RWZm?=
 =?utf-8?Q?1a63HBDxS6hbzof76zzOk3ieK?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6def9d8c-7baf-4bef-9a88-08da6f9640be
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5304.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2022 06:07:14.9910
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: G+BbyhVzWv3EIBWoGaknEI0aadmZl+YDz2P+kE4kvBSYMNdnXUjwVaoj7WpOVAX5ozl84irdoeWym1i8B/Hdrw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2622
X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 7/26/2022 10:53 PM, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2022 at 06:17:18PM +0530, Abhishek Sahu wrote:
>>  Thanks Alex for your thorough review of uAPI.
>>  I have incorporated all the suggestions.
>>  Following is the updated uAPI.
>>  
>>  /*
>>   * Upon VFIO_DEVICE_FEATURE_SET, allow the device to be moved into a low power
>>   * state with the platform-based power management.  Device use of lower power
>>   * states depends on factors managed by the runtime power management core,
>>   * including system level support and coordinating support among dependent
>>   * devices.  Enabling device low power entry does not guarantee lower power
>>   * usage by the device, nor is a mechanism provided through this feature to
>>   * know the current power state of the device.  If any device access happens
>>   * (either from the host or through the vfio uAPI) when the device is in the
>>   * low power state, then the host will move the device out of the low power
>>   * state as necessary prior to the access.  Once the access is completed, the
>>   * device may re-enter the low power state.  For single shot low power support
>>   * with wake-up notification, see
>>   * VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY_WITH_WAKEUP below.  Access to mmap'd
>>   * device regions is disabled on LOW_POWER_ENTRY and may only be resumed after
>>   * calling LOW_POWER_EXIT.
>>   */
>>  #define VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY 3
>>  
>>  /*
>>   * This device feature has the same behavior as
>>   * VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY with the exception that the user
>>   * provides an eventfd for wake-up notification.
> 
> It feels like this should be one entry point instead of two.
> 
> A flag "automatic re-sleep" and an optional eventfd (-1 means not
> provided) seems to capture both of these behaviors in a bit clearer
> and extendable way.
> 
> Jason

 We discussed about that in the earlier version of the patch series.
 Since we have different exit related handling, so to avoid confusion
 we proceeded with 2 separate variants for the low power entry. Also,
 we don't need any parameter for the first case.

 But, I can do the changes to make a single entry point, if we conclude
 for that. 

 From my side, I have explored how the uAPI looks like if
 we go with this approach.

 /*
  * Upon VFIO_DEVICE_FEATURE_SET, allow the device to be moved into a low power
  * state with the platform-based power management.  Device use of lower power
  * states depends on factors managed by the runtime power management core,
  * including system level support and coordinating support among dependent
  * devices.  Enabling device low power entry does not guarantee lower power
  * usage by the device, nor is a mechanism provided through this feature to
  * know the current power state of the device.  If any device access happens
  * (either from the host or through the vfio uAPI) when the device is in the
  * low power state, then the host will move the device out of the low power
  * state as necessary prior to the access.  Once the access is completed, the
  * device re-entry to a low power state will be controlled through
  * VFIO_DEVICE_LOW_POWER_REENTERY_DISABLE flag.
  *
  * If LOW_POWER_REENTERY_DISABLE flag is not set, the device may re-enter the
  * low power state.  Access to mmap'd device regions is disabled on
  * LOW_POWER_ENTRY and may only be resumed after calling LOW_POWER_EXIT.
  *
  * If LOW_POWER_REENTERY_DISABLE flag is set, then user needs to provide an
  * eventfd for wake-up notification.  When the device moves out of the low
  * power state for the wake-up, the host will not allow the device to re-enter
  * a low power state without a subsequent user call to LOW_POWER_ENTRY.
  * Access to mmap'd device regions is disabled on LOW_POWER_ENTRY and may only
  * be resumed after the low power exit.  The low power exit can happen either
  * through LOW_POWER_EXIT or through any other access (where the wake-up
  * notification has been generated).  The access to mmap'd device regions will
  * not trigger low power exit.
  *
  * The notification through the provided eventfd will be generated only when
  * the device has entered and is resumed from a low power state after
  * calling this device feature IOCTL.  A device that has not entered low power
  * state, as managed through the runtime power management core, will not
  * generate a notification through the provided eventfd on access.  Calling the
  * LOW_POWER_EXIT feature is optional in the case where notification has been
  * signaled on the provided eventfd that a resume from low power has occurred.
  *
  * The wakeup_eventfd needs to be valid only if LOW_POWER_REENTERY_DISABLE
  * flag is set, otherwise, it will be ignored.
  */
 #define VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY 3
 
 struct vfio_device_low_power_entry_with_wakeup {
 	__u32 flags;
 #define VFIO_DEVICE_LOW_POWER_REENTERY_DISABLE	(1 << 0)
 	__s32 wakeup_eventfd;
 };
 
 /*
  * Upon VFIO_DEVICE_FEATURE_SET, disallow use of device low power states as
  * previously enabled via VFIO_DEVICE_FEATURE_LOW_POWER_ENTRY device feature.
  * This device feature IOCTL may itself generate a wakeup eventfd notification
  * if the device had previously entered a low power state with
  * VFIO_DEVICE_LOW_POWER_REENTERY_DISABLE flag set.
  */
 #define VFIO_DEVICE_FEATURE_LOW_POWER_EXIT 4

 Thanks,
 Abhishek