Re: [Qemu-devel] [PATCH v2 3/8] nvdimm acpi: introduce _FIT

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 10/13/2016 09:33 PM, Igor Mammedov wrote:
> On Wed, 12 Oct 2016 16:20:07 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> On 10/11/2016 07:49 PM, Igor Mammedov wrote:
>>> On Mon, 10 Oct 2016 21:09:30 +0800
>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>> On 10/10/2016 08:51 PM, Igor Mammedov wrote:
>>>>> On Sat, 8 Oct 2016 15:17:14 +0800
>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>
>>>>>> On 09/30/2016 09:14 PM, Igor Mammedov wrote:
>>>>>>> On Fri, 12 Aug 2016 14:54:05 +0800
>>>>>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>>>>>
>>>>>>>> _FIT is required for hotplug support, guest will inquire the updated
>>>>>>>> device info from it if a hotplug event is received
>>>>>>>>
>>>>>>>> As FIT buffer is not completely mapped into guest address space, so a
>>>>>>>> new function, Read FIT whose function index is 0xFFFFFFFF, is reserved
>>>>>>>> by QEMU to read the piece of FIT buffer. The buffer is concatenated
>>>>>>>> before _FIT return
>>>>>>> Only issuer of UUID 2F10E7A4-9E91-11E4-89D3-123B93F75CBA can reserve
>>>>>>> 0xFFFFFFFF for some purposes.
>>>>>>> So spec should be amended first or custom generated UUID should be used.
>>>>>>
>>>>>> Okay.
>>>>>>
>>>>>> I will change the changelog to reflect this fact and move the spec update
>>>>>> to this patch.
>>>>> under spec, I've meant ACPI spec where this UUID is declared
>>>>
>>>> Er. ACPI spec just said that "0xFFFF is reserved", not sure it will be used
>>>> in the future.
>>>>
>>>> I'd prefer to custom-generated UUID, however, currently the UUID is checked
>>>> in OSPM, i.e, QEMU is not able to distinguish other UUIDs,
>>> I'd go with custom-generated UUID
>>>
>>>> so how about
>>>> drop the UUID check in ACPI and pass the UUID info to QEMU?
>>> It's a bit late to do so as it would be qemu-guest ABI change and
>>> one would need to maintain old and new protocol.
>>>
>>
>> Okay.
>>
>> How about extract 16 bits from 'handle' filed in NvdimmDsmIn buffer and use
>> it to indicate different UUIDs, i,e:
>> struct NvdimmDsmIn {
>>      uint16_t handle;
>>      uint16_t uuid_type;
>>      uint32_t revision;
>>      uint32_t function;
>>      /* the remaining size in the page is used by arg3. */
>>      union {
>>          uint8_t arg3[4084];
>>      };
>> } QEMU_PACKED;
>> typedef struct NvdimmDsmIn NvdimmDsmIn;
>>
>> For this case, we set uuid_type = 1 to indicate the UUID is the one used
>> for RFIT.
>>
>> u16 for 'handle' is large enough as QEMU supports 255 memory devices.
> I wouldn't do above as it needlessly complicates qemu<->OSPM
> protocol.
> Since handle is completely internal QEMU thing we can just reserve 0xFFFFFFFF value
> in docs/specs/acpi_nvdimm.txt  for RFIT and keep UUID checks only on ASL side.
>
> and on QEMU side add check to guaranty that auto generated handle for DIMMs
> would never go into reserved range.
>

Okay, I agree. As we may need to support multiple UUIDs in the future. I'd
like to document the rule for handle reservation:
   The handle is completely QEMU internal thing, the value in range [0, 0xFFFF]
   indicates nvdimm device (O means nvdimm root device named NVDR), the values
   out of this region are reserved by other purpose.

   Current reserved handles:
   0x10000 is reserved for RFIT.