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

Re: [Xen-devel] [PATCH v3 for 4.13] x86/e820: fix 640k - 1M region reservation logic


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
  • Date: Thu, 31 Oct 2019 11:45:40 +0000
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=sergey.dyasli@xxxxxxxxxx; spf=Pass smtp.mailfrom=sergey.dyasli@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Autocrypt: addr=sergey.dyasli@xxxxxxxxxx; keydata= mQINBFtMVHEBEADc/hZcLexrB6vGTdGqEUsYZkFGQh6Z1OO7bCtM1go1RugSMeq9tkFHQSOc 9c7W9NVQqLgn8eefikIHxgic6tGgKoIQKcPuSsnqGao2YabsTSSoeatvmO5HkR0xGaUd+M6j iqv3cD7/WL602NhphT4ucKXCz93w0TeoJ3gleLuILxmzg1gDhKtMdkZv6TngWpKgIMRfoyHQ jsVzPbTTjJl/a9Cw99vuhFuEJfzbLA80hCwhoPM+ZQGFDcG4c25GQGQFFatpbQUhNirWW5b1 r2yVOziSJsvfTLnyzEizCvU+r/Ek2Kh0eAsRFr35m2X+X3CfxKrZcePxzAf273p4nc3YIK9h cwa4ZpDksun0E2l0pIxg/pPBXTNbH+OX1I+BfWDZWlPiPxgkiKdgYPS2qv53dJ+k9x6HkuCy i61IcjXRtVgL5nPGakyOFQ+07S4HIJlw98a6NrptWOFkxDt38x87mSM7aSWp1kjyGqQTGoKB VEx5BdRS5gFdYGCQFc8KVGEWPPGdeYx9Pj2wTaweKV0qZT69lmf/P5149Pc81SRhuc0hUX9K DnYBa1iSHaDjifMsNXKzj8Y8zVm+J6DZo/D10IUxMuExvbPa/8nsertWxoDSbWcF1cyvZp9X tUEukuPoTKO4Vzg7xVNj9pbK9GPxSYcafJUgDeKEIlkn3iVIPwARAQABtChTZXJnZXkgRHlh c2xpIDxzZXJnZXkuZHlhc2xpQGNpdHJpeC5jb20+iQJOBBMBCgA4FiEEkI7HMI5EbM2FLA1L Aa+w5JvbyusFAltMVHECGwMFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQAa+w5JvbyuuQ JBAAry/oRK6m0I+ck1Tarz9a1RrF73r1YoJUk5Bw+PSxsBJOPp3vDeAz3Kqw58qmBXeNlMU4 1cqAxFxCCKMtER1gpmrKWBA1/H1ZoBRtzhaHgPTQLyR7LB1OgdpgwEOjN1Q5gME8Pk21y/3N cG5YBgD/ZHbq8nWS/G3r001Ie3nX55uacGk/Ry175cS48+asrerShKMDNMT1cwimo9zH/3Lm RTpWloh2dG4jjwtCXqB7s+FEE5wQVCpPp9p55+9pPd+3DXmsQEcJ/28XHo/UJW663WjRlRc4 wgPwiC9Co1HqaMKSzdPpZmI5D4HizWH8jF7ppUjWoPapwk4dEA7Al0vx1Bz3gbJAL8DaRgQp H4j/16ifletfGUNbHJR2vWljZ5SEf2vMVcdubf9eFUfBF/9OOR1Kcj1PISP8sPhcP7oCfFtH RcxXh1OStrRFtltJt2VlloKXAUggdewwyyD4xl9UHCfI4lSexOK37wNSQYPQcVcOS1bl4NhQ em6pw2AC32NsnQE5PmczFADDIpWhO/+WtkTFeE2HHfAn++y3YDtKQd7xes9UJjQNiGziArST l6Zrx4/nShVLeYRVW76l27gI5a8BZLWwBVRsWniGM50OOJULvSag7kh+cjsrXXpNuA4rfEoB Bxr7pso9e5YghupDc8XftsYd7mlAgOTCAC8uZme5Ag0EW0xUcQEQAMKi97v3DwwPgYVPYIbQ JAvoMgubJllC9RcE0PQsE6nEKSrfOT6Gh5/LHOXLbQI9nzU/xdr6kMfwbYVTnZIY/SwsLrJa gSKm64t11MjC1Vf03/sncx1tgI7nwqMMIAYLsXnQ9X/Up5L/gLO2YDIPxrQ6g4glgRYPT53i r6/hTz3dlpqyPCorpuF+WY7P2ujhlFlXCAaD6btPPM/9LZSmI0xS4aCBLH+pZeCr0UGSMhsX JYN0QRLjfsIDGyqaXVH9gwV2Hgsq6z8fNPQlBc3IpDvfXa1rYtgldYBfG521L3wnsMcKoFSr R5dpH7Jtvv5YBuAk8r571qlMhyAmVKiEnc+RonWl503D5bAHqNmFNjV248J5scyRD/+BcYLI 2CFG28XZrCvjxq3ux5hpmg2fCu+y98h6/yuwB/JhbFlDOSoluEpysiEL3R5GTKbxOF664q5W fiSObxNONxs86UtghqNDRUJgyS0W6TfykGOnZDVYAC9Gg8SbQDta1ymA0q76S/NG2MrJEOIr 1GtOr/UjNv2x4vW56dzX/3yuhK1ilpgzh1q504ETC6EKXMaFT8cNgsMlk9dOvWPwlsIJ249+ PizMDFGITxGTIrQAaUBO+HRLSBYdHNrHJtytkBoTjykCt7M6pl7l+jFYjGSw4fwexVy0MqsD AZ2coH82RTPb6Q7JABEBAAGJAjYEGAEKACAWIQSQjscwjkRszYUsDUsBr7Dkm9vK6wUCW0xU cQIbDAAKCRABr7Dkm9vK6+9uD/9Ld3X5cvnrwrkFMddpjFKoJ4yphtX2s+EQfKT6vMq3A1dJ tI7zHTFm60uBhX6eRbQow8fkHPcjXGJEoCSJf8ktwx/HYcBcnUK/aulHpvHIIYEma7BHry4x L+Ap7oBbBNiraS3Wu1k+MaX07BWhYYkpu7akUEtaYsCceVc4vpYNITUzPYCHeMwc5pLICA+7 VdI1rrTSAwlCtLGBt7ttbvaAKN4dysiN+/66Hlxnn8n952lZdG4ThPPzafG50EgcTa+dASgm tc6HaQAmJiwb4iWUOoUoM+udLRHcN6cE0bQivyH1bqF4ROeFBRz00MUJKvzUynR9E50F9hmd DOBJkyM3Z5imQ0RayEkRHhlhj7uECaojnUeewq4zjpAg2HTSMkdEzKRbdMEyXCdQXFnSCmUB 5yMIULuDbOODWo3EufExLjAKzIRWEKQ/JidLzO6hrhlQffsJ7MPTU+Hg7WxqWfn4zhuUcIQB SlkiRMalSiJITC2jG7oQRRh9tyNaDMkKzTbeFtHKRmUUAuhE0LBXP8Wc+5W7b3WOf2SO8JMR 4TqDZ0K06s66S5fOTW0h56iCCxTsAnRvM/tA4SERyRoFs/iTqJzboskZY0yKeWV4/IQxfOyC YwdU3//zANM1ZpqeE/8lnW/kx+fyzVyEioLSwkjDvdG++4GQ5r6PHQ7BbdEWhA==
  • Cc: Juergen Gross <jgross@xxxxxxxx>, "sergey.dyasli@xxxxxxxxxx >> Sergey Dyasli" <sergey.dyasli@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Thu, 31 Oct 2019 11:45:56 +0000
  • Ironport-sdr: eT4oDICyoQGnBLPPoh6+nd2klHjVl9wPUPBaRAV0wZQqdiVnyTgaF3Rc4o1+ay63djMmNsCB5Q jVckuYfdNuL6Jl/znB/PVJE3lfRy1Lq9pSbhCu7Rt06mMukfxhZ5y69Vzfm/5SdXjL1zqbCopN DMj7rOsAaUGDCXsaEDma6HuBSvfOmdRI5TlJ4DMRizHqsZKJ043q903EBFV9Ns9biSIdWujHXT lFC4Vln9e5FXcxvUh/xcu7X1Dxa21xt0vJEIkwvo0y1X2GivKFVf41MX7tO3yuN1rcfGfB2pm/ fe0=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 31/10/2019 11:07, Jan Beulich wrote:
> On 31.10.2019 11:56, Sergey Dyasli wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -274,6 +274,15 @@ static inline u32 phys_pkg_id(u32 cpuid_apic, int 
>> index_msb)
>>      return _phys_pkg_id(get_apic_id(), index_msb);
>>  }
>>  
>> +/*
>> + * Sometimes it's too early to use cpu_has_hypervisor which is available 
>> only
>> + * after early_cpu_init().
>> + */
>> +bool __init early_cpu_has_hypervisor(void)
>> +{
>> +    return cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR);
>> +}
> 
> OOI, did you consider introducing a more general early_cpu_has(),
> with X86_FEATURE_* passed as an argument?

I couldn't find any other users of cpuid_e{c,d}x(0x1), so I don't see
the point in doing it now.

>> @@ -318,9 +319,10 @@ static int __init copy_e820_map(struct e820entry * 
>> biosmap, unsigned int nr_map)
>>  
>>          /*
>>           * Some BIOSes claim RAM in the 640k - 1M region.
>> -         * Not right. Fix it up.
>> +         * Not right. Fix it up, but only when running on bare metal.
>>           */
>> -        if (type == E820_RAM) {
>> +        if ( !early_cpu_has_hypervisor() && type == E820_RAM )
>> +        {
>>              if (start < 0x100000ULL && end > 0xA0000ULL) {
>>                  if (start < 0xA0000ULL)
>>                      add_memory_region(start, 0xA0000ULL-start, type);
> 
> Seeing original line and lower context - aren't you corrupting
> previously reasonably consistent (Linux) style here? (This could
> be easily taken care of while committing, but I'd first like the
> point below be clarified.)

This file has mixed coding style and I don't have any preference here.

>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -5943,7 +5943,7 @@ const struct platform_bad_page *__init 
>> get_platform_badpages(unsigned int *array
>>      case 0x000806e0: /* erratum KBL??? */
>>      case 0x000906e0: /* errata KBL??? / KBW114 / CFW103 */
>>          *array_size = (cpuid_eax(0) >= 7 &&
>> -                       !(cpuid_ecx(1) & 
>> cpufeat_mask(X86_FEATURE_HYPERVISOR)) &&
>> +                       !early_cpu_has_hypervisor() &&
> 
> Strictly speaking this makes clear that in early_cpu_has_hypervisor()
> you should also check cpuid_eax(0) >= 1. We don't currently seem to
> object to running on a system with only basic leaf 0 available (we
> do object to there not being extended leaf 1). Andrew, do you have
> any clear opinion one way or the other?

What gave you that impression? early_cpu_init() and generic_identify()
do cpuid(0x1) unconditionally.

--
Thanks,
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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