Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Yin Fengwei' <fengwei.yin@intel.com>,
        "rjw@rjwysocki.net" <rjw@rjwysocki.net>,
        "lenb@kernel.org" <lenb@kernel.org>,
        "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] ACPI / processor_idle: use ndelay instead of io port
 access for wait
Thread-Topic: [PATCH v2] ACPI / processor_idle: use ndelay instead of io port
 access for wait
Thread-Index: AQHVgy8hI4gW7D6kAEmU4JIecWCIJ6dblHkg
Date:   Tue, 15 Oct 2019 11:48:15 +0000
Message-ID: <c9f3f4f93bb946f790fce4709253b359@AcuMS.aculab.com>
References: <20191015080404.6013-1-fengwei.yin@intel.com>
In-Reply-To: <20191015080404.6013-1-fengwei.yin@intel.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Yin Fengwei
> Sent: 15 October 2019 09:04
> In function acpi_idle_do_entry(), an ioport access is used for dummy
> wait to guarantee hardware behavior. But it could trigger unnecessary
> vmexit in virtualization environment.
> 
> If we run linux as guest and export all available native C state to
> guest, we did see many PM timer access triggered VMexit when guest
> enter deeper C state in our environment (We used ACRN hypervisor
> instead of kvm or xen which has PM timer emulated and exports all
> native C state to guest).
> 
> According to the original comments of this part of code, io port
> access is only for dummy wait. We could use busy wait instead of io
> port access to guarantee hardware behavior and avoid unnecessary
> VMexit.

You need some hard synchronisation instruction(s) after the inb()
and before any kind of delay to ensure your delay code is executed
after the inb() completes.

I'm pretty sure that inb() is only synchronised with memory reads.

...
> +	/* profiling the time used for dummy wait op */
> +	ktime_get_real_ts64(&ts0);
> +	inl(acpi_gbl_FADT.xpm_timer_block.address);
> +	ktime_get_real_ts64(&ts1);

That could be dominated by the cost of ktime_get_real_ts64().
It also need synchronising instructions.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)