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

[Igor Mammedov]

On Fri, 14 Oct 2016 15:43:50 +0800
Xiao Guangrong <address@hidden> wrote:

> On 10/13/2016 09:33 PM, Igor Mammedov wrote:
> > On Wed, 12 Oct 2016 16:20:07 +0800
> > Xiao Guangrong <address@hidden> wrote:
> >  
> >> On 10/11/2016 07:49 PM, Igor Mammedov wrote:  
> >>> On Mon, 10 Oct 2016 21:09:30 +0800
> >>> Xiao Guangrong <address@hidden> wrote:
> >>>  
> >>>> On 10/10/2016 08:51 PM, Igor Mammedov wrote:  
> >>>>> On Sat, 8 Oct 2016 15:17:14 +0800
> >>>>> Xiao Guangrong <address@hidden> wrote:
> >>>>>  
> >>>>>> On 09/30/2016 09:14 PM, Igor Mammedov wrote:  
> >>>>>>> On Fri, 12 Aug 2016 14:54:05 +0800
> >>>>>>> Xiao Guangrong <address@hidden> 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.
sounds fine to me