[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/hvm: Propagate real error information up through hvm_load()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Mon, 19 Jul 2021 14:22:21 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Ur1ch8104mNgcxHmhQ/BwTuaEUIV/QgFAHygXvrKcVM=; b=Y6tqRR7ZYXxlIAA75eZyPWANzggg40Ib/2FubZXVCUQyHeb8iugQvH9QB9l5+eXkCeiXYUApBu3FeIlqIVRCu43QsR4W1UN9gW/WtLEMDGQPUvFQ4mq8J3ihPSlLxUUsifoHCAfLSSx8r5H/0N87bZGZuliLq4ao5HJ2Sbx3wVlOqVpRiMB6kkLQVVS10V/1mZP11JPoFuvFUbn3pEAHkJpHVUpZnrIKB/Nus3zwOXOoqArl7QGHmmfkEYCVwoDLJmGgXDswfN3e85yBquu/kloODq6Rp6rsQUDHC/7nXZaxoUc2AofVwZblEyYcQH2s9jcXeV/3i4uDg1CYxCsyYQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=e6ESgjnmLtPaTX7oXqq8OhyMqd+zO85SdiDYSBtv0MrBh+AxMQvO3R0Pso5dQtqvyfDP0RDXA4rxh9haWp8yl25+XUoTbmFbrQONdiyUk/p4bGEqMa1AuMHNh1gBnXT411bV9sdMQdvcdZhZzrUMP/L1hyo2PMJM4fdeWDlYy8BQqt1Ay73incld6GfC974NrkkIqWrH2qLhIV25YeoNrw1dDLemwtg4JTptEsqEDiXtY12xqePnfJg4jMfU1uaAKZ+2qFRQus5/oMQ/KfCRgLgZxv3Se9ZJtINdFky6UkL9GNHAQWXC9Ezr0RRl0H1XMdWUuMVuYc4Ra2KMlfW4pg==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 19 Jul 2021 13:22:48 +0000
  • Ironport-hdrordr: A9a23:V2zwiqtxgx4ycxOrCAhHh0ZZ7skC9oMji2hC6mlwRA09TyXGra 6TdaUguiMc1gx8ZJhBo7C90KnpewK7yXcH2/huAV7EZniYhILIFvAf0WKG+Vzd8kLFh5VgPM tbAs1D4ZjLfCVHZKXBkXqF+rQbsaG6GcmT7I+0pRodLnAJGtRdBkVCe3+m+yVNNXl77PECZf 6hD6R81l2dkDgsH76G7i5vZZmzmzSHruOpXTc2QzocrCWehzKh77D3VzCewxclSjtKhZMv63 LMnQDV7riq96jT8G6d60bjq7Bt3PfxwNpKA8KBzuATNzXXkw6tIKBsQaeLsjwZqPymrHwqjN 7PiRE9ONkb0QKRQkiF5T/WnyXw2jcn7HHvjXeenHvYuMT8AAk3DsJQ7LgpPyfx2g4FhpVRwa hL12WWu958FhXbhhnw4NDOSlVDile0iWBKq59Xs1VvFa8lLJNBp40W+01YVL0aGjjh1YwhGO 5ySOnB+fdtd0+AZXyxhBgs/DWVZAV1Iv66eDlFhiTMuAImxUyRjnFoh/D3p01wsK7UEPJ/lr z52s0CrsA9cicUBZgNTtvpD/HHUVAk7Hr3QSuvyG/cZdY60kT22tXKCYUOlZWXkaMzve0Pcb T6IR9lXD0JCg3T4fPn5uwCzvmKehTmYQjQ
  • Ironport-sdr: 9BDeqnaHHdxpkRcLogvMcgRo7u1Pk7tzWJJuDkjf1O5/UqE8MHTbKPReTXinQidn84xOz7SDj0 SS3jmj1RH3IEqZRfzeDyY9hHU6oW5LCprOze26E6wyQpYWuyOCfUVvP3SW70YYochqbT6y1Bee siRkA7uirnV8WawdCy873f5e7002kdR7uJmUcd3myIMPn11AWFWAdvrvAlyhGZApZx6+IVHyzI 6vyYeiPXvPQQ088k/NLNOQX6w3vrgFraOpx6Ds0yrWMI6CyBWUrkOFvEV+dCDbo+M08mZJtggF vrXbpoXo9GI/aYv/DgcH0MNK
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 19/07/2021 13:46, Jan Beulich wrote:
> On 19.07.2021 13:14, Andrew Cooper wrote:
>> hvm_load() is currently a mix of -errno and -1 style error handling,
>> which
>> aliases -EPERM. This leads to the following confusing diagnostics:
>>
>> From userspace:
>> xc: info: Restoring domain
>> xc: error: Unable to restore HVM context (1 = Operation not
>> permitted): Internal error
>> xc: error: Restore failed (1 = Operation not permitted): Internal error
>> xc_domain_restore: [1] Restore failed (1 = Operation not permitted)
>>
>> From Xen:
>> (XEN) HVM10.0 restore: inconsistent xsave state (feat=0x2ff
>> accum=0x21f xcr0=0x7 bv=0x3 err=-22)
>> (XEN) HVM10 restore: failed to load entry 16/0
>>
>> The actual error was a bad backport, but the -EINVAL got converted to
>> -EPERM
>> on the way out of the hypercall.
>>
>> The overwhelming majority of *_load() handlers already use -errno
>> consistenty.
>> Fix up the rest to be consistent, and fix a few other errors noticed
>> along the
>> way.
>>
>> * Failures of hvm_load_entry() indicate a truncated record or other
>> bad data
>> size. Use -ENODATA.
> But then ...
>
>> @@ -421,8 +421,8 @@ static int pit_load(struct domain *d,
>> hvm_domain_context_t *h)
>> if ( hvm_load_entry(PIT, h, &pit->hw) )
>> {
>> - spin_unlock(&pit->lock);
>> - return 1;
>> + rc = -ENODEV;
>> + goto out;
>> }
> ... use -ENODATA here as well?

Hmm - that was intended to be ENODATA.  Will fix.

> Preferably with the adjustment
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

> I'll pick this up for backporting once I see it in the tree.

I don't know how much the call tree has changed over time.  Every
handler will need a quick check on each release.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.